-
Notifications
You must be signed in to change notification settings - Fork 343
Complete the FATES-CLM nitrogen coupling #3409
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
base: master
Are you sure you want to change the base?
Conversation
PASS ERS_D_Ld30.1x1_brazil.I2000Clm60FatesCrujraRs.derecho_intel.clm-FatesColdPRT2 FAIL ERS_D_Ld30.1x1_brazil.I2000Clm60FatesCrujraRs.derecho_intel.clm-FatesColdPRT2--clm-mimicsFatesCold--clm-nofireemis The latter needs "nofireemis" to work with Fates, but it then dumps core in line 1180 SoilBiogeochemDecompCascadeMIMICSMod.F90
...calculating these variables: nf_soil%decomp_npools_sourcesink_col nf_soil%fates_litter_flux
|
With the latest commit, I repeated the two earlier tests and added another to check whether answers have changed from the baseline: The latter fails in case2, after reading the restart file, with a N balance error. UPDATE
|
|
Enabled fixation and ran the same tests: PRT2 still b4b with the baseline. PASS The same tests with the code change for harvest (same results relative to the baseline). |
|
Worked on the next checkbox in the issue (#3378), submitted the same tests, and after some troubleshooting: |
These variables originate in fates, so this renaming requires the same renaming in fates; I will open the corresponding PR very soon
Notes:
|
|
Updated the fates paramfile (see next commit) and submitted these two again The first (LUH2) same as before (DIFF from baseline) since nothing changed for it.
|
|
Submitted the same 2 tests now with the error check suggested in NGEET/fates#1454 (see corresponding commit NGEET/fates@97839f1 in NGEET/fates#1472)
UPDATE Repeating with 2025/12/10 updates:
|
|
The reason for the last commit is explained in #2653 (comment). |
Update crop parameters and maturity requirements slevis resolved conflicts: src/fates <-- So far I have not changed .gitmodules to point to my branch src/soilbiogeochem/SoilBiogeochemCompetitionMod.F90
FATES JSON parameter files These changes accommodate a JSON format to the FATES parameter file. Documentation has been updated in the FATES User's Guide: https://fates-users-guide.readthedocs.io/en/latest/user/Parameter-File.html fates was updated from sci.1.88.0_api.42.0.0 to sci.1.89.0_api.43.0.0 ESCOMP and NGEET PRs: ESCOMP#3570 NGEET/fates#1493 slevis resolved conflicts: cime_config/testdefs/testmods_dirs/clm/FatesColdPRT2/shell_commands cime_config/testdefs/testmods_dirs/clm/FatesColdPRT2/user_nl_clm src/fates
|
After the latest updates, I'm submitting these tests with ./create_test: LUH2 diffs from fates-sci.1.89.0_api.43.0.0-ctsm5.4.005: PRT2 diffs in the same 38 fields in base.cprnc.out as in cprnc.out.
|
| if ( ($parteh_mode == 2) && ($suplnitro !~ /NONE/) && not &value_is_true( $nl_flags->{'use_fates_sp'}) ) { | ||
| $log->fatal_error("supplemental Nitrogen (suplnitro) is NOT set to NONE, FATES is on, " . | ||
| "and FATES-SP is not active, but fates_parteh_mode is 2, so Nitrogen is active; " . | ||
| "change suplnitro back to NONE"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this new check because in parteh_mode = 2 we want to allow either NONE or ALL suplnitro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or replace check with not allowing parteh_mode = 2 and sp mode together.
|
|
||
| my $parteh_mode = $nl->get_value('fates_parteh_mode'); | ||
| if ( ($parteh_mode == 1) && ($suplnitro !~ /ALL/) && not &value_is_true( $nl_flags->{'use_fates_sp'}) ) { | ||
| $log->fatal_error("supplemental Nitrogen (suplnitro) is NOT set to ALL, FATES is on, " . | ||
| "and FATES-SP is not active, but fates_parteh_mode is 1, so Nitrogen is not active. " . | ||
| "Change suplnitro back to ALL"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here remove the sp condition because irrelevant.
Description of changes
For now see the issue #3378
Corresponding mods on the FATES side:
NGEET/fates#1472
Nutrient enabled FATES handbook
FATES CLM N coupling
Specific notes
Contributors other than yourself, if any:
@rgknox @adrifoster @wwieder
CTSM Issues Fixed (include github issue #):
#3378
Are answers expected to change (and if so in what way)?
Any User Interface Changes (namelist or namelist defaults changes)?
Does this create a need to change or add documentation? Did you do so?
Testing performed
...with the first two commits in this PR:
Later comments point out that these two tests were inadequate at catching problems, and that I switched to two other tests.