Get FATES and carbon_only test-suite tests to pass with MIMICS active #1643
Conversation
Values by pft from Table 1 in Brovkin et al. (2012): https://bg.copernicus.org/articles/9/565/2012/bg-9-565-2012.pdf This hardwiring will become unnecessary when we calc. ligninNratioAvg the same way for FATES and non FATES cases.
|
Ran cheyenne test suite with MIMICS as the default bgc option: |
|
@rgknox thank you for alerting me to the fact that annsum_npp gets passed from FATES to the HLM only if At your recommendation I have disallowed FATES-MIMICS with Default test |
ekluzek
added
tag: enh - new science
enhancement
Not tested; I will post info about testing next.
cheyenne PASS izumi OK
slevis-lmwg
added
the
PR status: ready
I had mostly reverted the corresponding test to how it was before this PR but had missed this...
This eliminates the need for changes on the FATES side and it also eliminates the restart test failure.
... soilbiogeochem_carbonflux to avoid duplication
Tangential to this PR: It affects comments, plus makes the CWD HR history variable active, so Will and I decided to slide it in
This comment was marked as outdated.
This comment was marked as outdated.
ERP_P144x2_Ld30.f45_f45_mg37.I2000Clm50FatesRs.cheyenne_intel.clm-mimicsFatesColdDef
Refactor NutrientCompetition / CNAllocation to provide hooks for AgSys Major refactor of NutrientCompetition / CNAllocation to provide hooks for AgSys crop model: separates the NutrientCompetition modules into pieces based on (1) consolidating duplicate code between the Clm45 and FlexibleCN versions, and (2) separating pieces that will vs. won't be used for crop patches when running with the upcoming AgSys crop model. I have restored the old CNAllocationMod, with some of the responsibilities that it used to have. (I'm not sure it's appropriate to have the calculation of gpp and maint resp in CNAllocationMod, but I left it there because it has always been combined with the allocation code, including back when we had a separate CNAllocationMod.) Resolved conflicts: doc/ChangeLog doc/ChangeSum src/biogeochem/CNDriverMod.F90
|
@ekluzek recommends adding the threading test that I used above to the test-suite, in the aux-clm and fates lists. |
|
@slevisconsulting @glemieux and I went over this, this morning. Since the copy_fates_var is really specific to the clmfates_interface model I think it's important to keep it local there. So we went over how to make it a local module level variable, and it looks like that's working now. Since, fates-SP mode has the biggest changes to the CTSM side of running FATES it makes sense to have a threading test specific for it. |
I changed it from izumi to cheyenne because I wasn't sure whether it would be ok on izumi. I put it on the aux_clm and fates test lists.
|
Test suites: I think the PR is ready. Pls let me know if I'm forgetting something... |
ekluzek
dismissed
rgknox’s stale review
I think all of the concerns were addressed
Description of changes
To pass with MIMICS active, FATES and carbon_only tests need calculations of:
ligninNratioAvgandannsum_npp_col, both currently declared and calculated inCNVegCarbonFluxType.F90.ligninNratioAvgis a function ofleafc_to_litter_patch, frootc_to_litter_patch, soilbiogeochem_cwdc_col, soilbiogeochem_cwdn_col, soilbiogeochem_decomp_cascade_ctransfer_col.I got both FATES and carbon_only tests to pass by declaring local copies (in MIMICS) of
ligninNratioAvgandannsum_npp_col.Specific notes
Contributors other than yourself, if any:
@wwieder @rgknox @ckoven @ekluzek
CTSM Issues Fixed (include github issue #):
Fixes #1636
Are answers expected to change (and if so in what way)?
Mostly not applicable. MIMICS is a new option and we're testing it here as if it were the default option. However, actual MIMICS tests introduced in dev074 should be bfb, unless they end up with roundoff diffs due to changes in the order of certain calculations.
Any User Interface Changes (namelist or namelist defaults changes)?
No.
Update:
Turns out that carbon_only was failing due to annsum_npp < 0, which caused the MIMICS sqrt of this quantity to fail. The fix for this is different than I originally thought, but I am still addressing it in this PR along with the fix for the FATES failures.
Issue discovered while working on this PR:
For what I can tell, c3psn for sorghum and millet are set to 1 in the param files, and I'm pretty sure they should be 0 (ie not c3 but c4 photosynthesis). I suspect I made this mistake when I first introduced these cfts. I will open an issue to get confirmation from others that this is a mistake.
Testing performed, if any:
With MIMICS active, these tests now pass with the code changes committed this far:
ERP_D_P36x2_Ld3.f10_f10_mg37.I2000Clm50BgcCru.cheyenne_intel.clm-cn_conlySMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.cheyenne_gnu.clm-FatesColdDef