ctsm5.4.030: Add a FATES namelist option for land use transition logic#3728
ctsm5.4.030: Add a FATES namelist option for land use transition logic#3728samsrabin merged 21 commits intoESCOMP:masterfrom
Conversation
glemieux
left a comment
There was a problem hiding this comment.
This needs a check in the build namelist to make sure that landuse is not accidentally set off by the user in the namelist. I'll add this check. Otherwise this looks good.
Change to using the ifx compiler and some more fixes Updates ccs_config to 1.0.75 which has the new ifx compiler in use for derecho_intel, as well as updating to using ESMF8.9.0. This exposed problems in the code on two fronts when running with DEBUG mode: Reliance on short-circuiting in if statements Problems with NaN's, in datasets especially _FillValue For the former, we changed code to break up if statements so this would work. And with the latter we point to new datasets with the NaN's handled. mosart and cdeps code also had short-circuiting problems. @samsrabin created a script to examine all of the files in namelist_defaults_ctsm.xml and in the testmod directories to check if there were problems with NaN's in _FillValues. The modified files here all were the result of the first pass of that script. The new files all have a ".no_nan_fill.nc" ending on them. Includes updating submodules: ccs_config, cdeps, mosart
|
FYI, issue to track adding the new namelist option to the FATES user's guide is here: NGEET/fates-users-guide#110 |
There was a problem hiding this comment.
This is great thanks @JessicaNeedham. I have comments/questions on how best to document this for CTSM users so they have ways of looking up how this works.
There are some things that will need to happen before this comes in. This will come to master as it will include a FATES update, which normally has answer changes that come with it.
- @glemieux makes a PR to this one with a change he sees needs to be done
- Add in the FATES tag to the .gitmodule file
- Resolve conversations
- Decide who will do the testing and finalize the tag
- Go through the normal testing/tagging process
|
@ekluzek, my PR to @JessicaNeedham branch has been merged. Would you give it another review when you get a chance and see if my changes look good to you? I think this may be ready for testing (which I'm willing to run). |
Transfer biophysical variables for output with FATES dimensions This pull request updates the high frequency wrapper procedure in the interface to pass biophysical CLM variables to be output along FATES dimensions. The FATES land use x pft dimension is also added to the history module. The FATES base land use test module adds the FATES history noted above to the test output.
|
Per SE meeting today: Don't go with @rgknox's suggestion of moving things to the namelist JSON. |
|
FYI @ekluzek the fates-side testing is looking good so far: NGEET/fates#1489 (comment). Where in the queue do you think this is? |
ekluzek
left a comment
There was a problem hiding this comment.
I have only one change that I think is important. Which is to remove the checking for valid values in CLMBuildNamelist.pm, since it's already done based on the valid_values in the namelist_definition_ctsm.xml file.
I suggest a change to the description in namelist_definition_ctsm.xml. And I was wondering about how the Fortran code reacts to bad values of the namelist flag.
So all things that are small.
@rgknox and @glemieux I'm not sure who's going to bring this in. But, if there are any questions on what I ask about we could meet to go over it.
ekluzek
left a comment
There was a problem hiding this comment.
Approving so this can go ahead. I resolved the previous conversations, and just added one to remove the extra testing of values in CLMBuildNML.pm.
Merge b4b-dev to master - Various documentation updates - Update submodules to ones in cesm3_0_alpha08l - Bugfix for FatesSp compiled with intel PR ESCOMP#3894
|
Final |
|
Regression testing on |
|
Regression testing on Results: |
|
Changelog update. This is ready to integrate @samsrabin. |
|
Removed the namelist build check per Erik's request. Retested Results: |
Description of changes
This change is coordinated with FATES PR NGEET/fates#1489.
Together they move a hard coded value in FATES, which controls vegetation mortality during transition between land use classes, to a namelist option.
Specific notes
Contributors other than yourself, if any:
CTSM Issues Fixed (include github issue #):
Are answers expected to change (and if so in what way)?
This PR sets the default to option 1 which is different than the current hard coded value of 4. Setting the value to 4 would be equivalent to existing code.
Any User Interface Changes (namelist or namelist defaults changes)?
Yes - see above.
Does this create a need to change or add documentation? Did you do so?
On the FATES side. Not done yet.
Testing performed, if any:
Compiles and runs with NorESM-CTSM-FATES.
NOTE: Be sure to check your coding style against the standard
(https://github.com/ESCOMP/ctsm/wiki/CTSM-coding-guidelines) and review
the list of common problems to watch out for
(https://github.com/ESCOMP/CTSM/wiki/List-of-common-problems).