Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes for when INTERNAL_FILE_NML is not used #23

Closed
wants to merge 1 commit into from
Closed

Changes for when INTERNAL_FILE_NML is not used #23

wants to merge 1 commit into from

Conversation

danholdaway
Copy link
Collaborator

At the JCSDA we don't use the INTERNAL_FILE_NML compiler definition since we need to use different resolutions in the same executable. After the most recent merging from GFDL this capability stopped working. A couple of simple changes to make it work again.

In test_cases.f90 there are some missing includes.

In fv_control.f90 GNU compilers don't like trying to open an already open file.

@danholdaway
Copy link
Collaborator Author

I can't assign reviewers, could someone add the appropriate people please?

@climbfuji
Copy link

I can't assign reviewers, could someone add the appropriate people please?

Life is not that easy ;-) There are a few commits, at least one that is currently worked on, in the queue to happen before this can get tested. Usually reviewers are added at the time the tests are run in this setup. Not ideal, but that's how it has been until now. Given the small number of changes I would suggest to have reviewers look at it earlier so that when it is ready and the tests pass it can be committed. Only @junwang-noaa or @DusanJovic-NOAA can add reviewers. I suggest @bensonr.

@danholdaway
Copy link
Collaborator Author

Thanks for the info @climbfuji. No particular rush since we typically work from our fork. Just wanted to make sure the changes propagate upstream at some point.

@danholdaway
Copy link
Collaborator Author

p.s. thanks for fixing the incompatibility with GNU that was introduced.

@climbfuji
Copy link

p.s. thanks for fixing the incompatibility with GNU that was introduced.

For your information, in the last commit I set up Hera with gnu-9.3.0 as Tier-1 platform, which means that any commit to go in must pass the regression tests with this compiler as well. Thus, the days of commits breaking GNU code are history.

@arunchawla-NOAA @ligiabernardet FYI

@danholdaway
Copy link
Collaborator Author

p.s. thanks for fixing the incompatibility with GNU that was introduced.

For your information, in the last commit I set up Hera with gnu-9.3.0 as Tier-1 platform, which means that any commit to go in must pass the regression tests with this compiler as well. Thus, the days of commits breaking GNU code are history.

@arunchawla-NOAA @ligiabernardet FYI

That's great news, thanks. Should save us some time downstream. Our CI includes Intel, Clang and GNU so we need that working.

@climbfuji
Copy link

p.s. thanks for fixing the incompatibility with GNU that was introduced.

For your information, in the last commit I set up Hera with gnu-9.3.0 as Tier-1 platform, which means that any commit to go in must pass the regression tests with this compiler as well. Thus, the days of commits breaking GNU code are history.
@arunchawla-NOAA @ligiabernardet FYI

That's great news, thanks. Should save us some time downstream. Our CI includes Intel, Clang and GNU so we need that working.

I usually test on my mac with clang+gfortran, but this is of course not a tier-1 (pre-commit) testing, only when I work on/with the code. But gcc+gfortran catches most of the issues.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jun 23, 2020 via email

@danholdaway
Copy link
Collaborator Author

@junwang-noaa my understanding is that with the compiler definition set the input.nml file is read once when fms is initialized. In data assimilation we have to create several geometries potentially of different resolution. E.g. the background is the high resolution C768 while the increment is the lower C384. If doing 4DVar we even have increments at C48, C96, C192 etc as well. That all needs to be done in-core. So in the C++ level of JEDI we actually move different input.nml files in and out of the directory as we're running and read several of them. Unless I'm misunderstanding things -DINTERNAL_FILE_NML would not allow for that.

@danholdaway
Copy link
Collaborator Author

danholdaway commented Jun 23, 2020

@junwang-noaa I would also note that GMAO (unless things have changes recently @wmputman?) also do not use -DINTERNAL_FILE_NML. For GEOS it's because npx, npy and layout are provided by geos and fv_init is broken into two steps.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jun 23, 2020 via email

@danholdaway
Copy link
Collaborator Author

OK, thanks for sending those numbers @junwang-noaa. We'll look into having the read be done by one processor if we find it's an issue. I assumed that's what it was doing since it's read by an mpp routine so good to know that it's not.

@bensonr
Copy link
Collaborator

bensonr commented Jun 23, 2020 via email

@danholdaway
Copy link
Collaborator Author

Thanks @bensonr, good to know about that functionality. I'll try that out in my geometry generation code.

@danholdaway danholdaway deleted the dev/jcsda branch July 22, 2020 15:11
aerorahul pushed a commit that referenced this pull request Jul 17, 2021
* Add LICENSE.md

* Renamed driver directories and removed the null version of the nh extensions for the public release

* Update license header for FV3

* Added a README.md file

* Replace GPL header with LGPL header

* fixed license header in every file and added it to those without it

* Master test (#17)

* commit of new version of dycore from Weather and Climate Dynamics Group at GFDL

* updated versions of GFDL-specific files from dev/gfdl

* updated README.md with current release information

* cleaned up a few lines in fv_dynamics

* new file RELEASE.md with release notes documenting differences between this and the last release

* updated RELEASE.md message

* hand merge of diagnostic updates

* remove trailing spaces from sources

* updates to merge some GFDL specific updates into this public release

* Master test (#18)

* commit of new version of dycore from Weather and Climate Dynamics Group at GFDL

* updated versions of GFDL-specific files from dev/gfdl

* updated README.md with current release information

* cleaned up a few lines in fv_dynamics

* new file RELEASE.md with release notes documenting differences between this and the last release

* updated RELEASE.md message

* hand merge of diagnostic updates

* remove trailing spaces from sources

* updates to merge some GFDL specific updates into this public release

* updated README.md

* updated GFDL_tools/fv_cmip_diag to be consistent with dev/gfdl branch

* Bug fix for two-way nest updating (#21)

* remove trailing whitespace and any tabs

* update the RELEASE.md with the FV3 technical memorandum

* semantic fix in RELEASE.md

* adds default values for nest_*offsets in fv_control

breaks up a too long line in fv_nesting.F90

* change default value of nestupdate to 7

Co-authored-by: Seth Underwood <Seth.Underwood@noaa.gov>
Co-authored-by: lharris4 <53020884+lharris4@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants