Allow understory leaves to have a different turnover rate than canopy leaves

[JessicaNeedham]

To try to increase understory survival and LAI, this PR allows cohorts in the understory to have leaf turnover rates that are different (longer) than leaf turnover rates in the canopy.

Collaborators:

@ckoven @jennykowalcz @mpaiao

Expectation of Answer Changes:

This will be answer changing. Understory cohorts will have reduced turnover of leaves, which should decrease carbon starvation mortality.

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

• The in-code documentation has been updated with descriptive comments
• The documentation has been assessed to determine if updates are necessary

Integrator

• FATES PASS/FAIL regression tests were run
• Evaluation of test results for answer changes was performed and results provided

Documentation

• Technical Note update:
• User's Guide update:

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

[jennykowalcz]

I tried this out with some pan-Amazon simulations with a single evergreen PFT, setting the understory leaf lifespan to 9 years in the test case and 3 years in the control case (canopy leaf lifespan = 3 years in both cases). The longer understory leaf lifespan increases understory LAI, understory and total biomass, and understory biomass residence time (slightly). Carbon starvation mortality decreases. Understory leaf turnover decreases, as we would expect. Changes to canopy LAI are negligible. GPP and respiration both increase, with respiration increasing more than GPP, resulting in decreased NPP and carbon use efficiency.
Understory leaf lifespan.pdf

So, this PR has the intended effects of increasing understory LAI, decreasing carbon starvation mortality, and increasing understory longevity. One thing that came up in conversation with @JessicaNeedham, @ckoven, and @mpaiao is if it would be useful to also have separate parameters for canopy and understory maximum mortality rate from carbon starvation (fates_mort_scalar_cstarvation).

[mpaiao]

@JessicaNeedham Thanks for submitting this pull request. I went through and it looks great, I just made a couple of very minor suggestions.

[mpaiao]

Does this mean that the leaf longevity for understory must be always defined (as opposed to having them the same by default and only applying different longevities if the user wants it)? I'm fine either way, just curious about it.

[JessicaNeedham]

Yes, there has to be a value in the parameter file. But it defaults to the same as canopy trees so it is only different if a user wants it to be.

[JessicaNeedham]

These are now the same

[mpaiao]

Suggested change

	if ( (prt_params%leaf_long(ipft,aclass_sen_id) > nearzero ) .and. &
	int(prt_params%evergreen(ipft))==itrue ) then
	if ( (prt_params%leaf_long(ipft,aclass_sen_id) > nearzero ) .and. &
	prt_params%evergreen(ipft) == itrue ) then

We don't need the int() anymore because we had switched the evergreen and deciduous flags to be all integers.

[jennykowalcz]

This looks great @JessicaNeedham, and in the simulations I have run with this branch it seems to work as expected, i.e. understory biomass turnover time decreases with increased understory leaf lifespan

[rgknox]

Tests with JessicaNeedham#5 are b4b with a special configuration. Note in that PR that update to the parameter file from the grass allometry and nutrient uptake PRs are combined. In this special configuration I only updated the parameter file to have the new understory turnover field, but kept all parameters as the previous default.

/glade/derecho/scratch/rgknox/tests_0814-204825de

Next step is to test with the parameter file changes defined in the pull request (ie the complete pull request). This will not be b4b.

[rgknox]

I wanted to keep a record of @XiulinGao 's changes to the parameter file, so that we could double check to see that they were included here:

This diff (fates_params_default.cdl) should contain the changes:
https://github.com/NGEET/fates/compare/sci.1.76.3_api.35.1.0...rgknox:xiulin-params-old?expand=1

[glemieux]

Regression testing against fates-sci.1.78.2_api.36.0.0-ctsm5.2.029 after merging up ESCOMP/CTSM#2700 to the latest tag is returning two unexpected RUN fails in hydro. The last regression tests were run against ctsm5.2.021 for reference:

FAIL ERS_D_Ld5.1x1_brazil.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdHydro RUN time=130
FAIL SMS_Lm3_D_Mmpi-serial.1x1_brazil.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdHydro RUN time=73

Both are failing in on the same line:

 18 forrtl: error (73): floating divide by zero
 19 Image              PC                Routine            Line        Source
 20 libpthread-2.31.s  00001470B4ECA8C0  Unknown               Unknown  Unknown
 21 cesm.exe           00000000022A4BB7  fatesplanthydraul        3008  FatesPlantHydraulicsMod.F90
 22 cesm.exe           0000000001B75BC4  edcohortdynamicsm         260  EDCohortDynamicsMod.F90
 23 cesm.exe           0000000001BD8581  edinitmod_mp_init        1308  EDInitMod.F90
 24 cesm.exe           0000000001BCC2D7  edinitmod_mp_init         884  EDInitMod.F90
 25 cesm.exe           0000000000B9175D  clmfatesinterface        2179  clmfates_interfaceMod.F90
 26 cesm.exe           0000000000AACED6  clm_initializemod         757  clm_initializeMod.F90
 27 cesm.exe           00000000009AEB44  lnd_comp_nuopc_mp         659  lnd_comp_nuopc.F90

Note that this is using parameter file lnd/clm2/paramdata/fates_params_api.36.1.0_12pft_c240814.nc which does not include the parameter file changes from #1236 yet. I'm going to attempt to rerun these tests specifically with the arctic shrub parameter file update next.

[glemieux]

Combining the updates from #1236 and rerunning the regression tests results in the same failure modes as seen above. Double checking the fates-sci.1.78.2_api.36.0.0-ctsm5.2.029 baselines, I confirmed that the hydro tests were passing per the TestStatus output, which suggests the issue isn't due to recent clm updates.

[glemieux]

I've isolated the hydro failure mode here to having to do with the parameter file updates from #1206. I created a branch based off of main that has a patch file to update the grass parameter values only on my repo: https://github.com/glemieux/fates/tree/pr1206-param-patchonly. Testing the ERS hydro test mod with this updated param file on fates main and ctsm5.2.029 results in the same failure mode. I'm running a full test suite as well for completeness sake.

@rgknox and @XiulinGao could you take a look at the failure mode noted above in #1136 (comment) when you have a moment?

I'm also running a test back on the original tag for #1206.

UPDATE: testing with the grass-only parameter file using sci.1.78.0_api.36.0.0 fails in the same manner.
Location: /glade/derecho/scratch/glemieux/ctsm-tests/tests_0927-145551de

[XiulinGao]

Try switch the sapwood allometry model to the original model (which should be 1) and see if the failure still persist. My hypothesis is that the new grass sapwood model skipped calculating sapwood area, and theoretically sapwood area is related to plant hydro.

[glemieux]

Try switch the sapwood allometry model to the original model (which should be 1) and see if the failure still persist. My hypothesis is that the new grass sapwood model skipped calculating sapwood area, and theoretically sapwood area is related to plant hydro.

@XiulinGao that worked, the hydro test passes now. I'll add some additional diagnostics to see if I can add more detail to confirm your hypothesis.

[glemieux]

@XiulinGao I can confirm that when it fails the a_sapwood is undefined (an extremely small number on the order ofE-324) and that the kmax_upper, kmax_lower and kmax_node values are all evaluating as zero resulting in the crash. Does this mean we need a grass specific way to calculate the sapwood area or can we use the calculation made in bsap_ltarg_slatop to get the area? Or do we need a grass-specific way to calculate the kmax_ values?

[XiulinGao]

@glemieux Thank you for confirming that! I'll have to take a look at the hydro module code to see what is exatcly required and then from there we can decide if we'll need to switch back to the old sapwood model or come up with a different way to do it for grasses.

[rgknox]

This is how I interpret the probable issue...

The calculation of maximum hydraulic conductance through the stem is highly dependent on the cross sectional area of the organ. We ask for the cross sectional area with the call to bsap_allom:

https://github.com/NGEET/fates/blob/main/biogeophys/FatesPlantHydraulicsMod.F90#L2953-L2954

But, as you point out, this is undefined. I checked through the new allometry code for allom_smode = 2, and as far as I can tell, we don't return that cross sectional area: https://github.com/NGEET/fates/blob/main/biogeochem/FatesAllometryMod.F90#L1048C2-L1061C14

@XiulinGao , can we calculate a cross sectional area for the plant with mode 2?

[XiulinGao]

@rgknox @glemieux Great! I have two proposals:

1. since all stem biomass of grasses are considered as sapwood, knowing basal diameter (d) of a greass and by assuming the architecture of a grass is a cylinder we can calculate the cross sectional area for grass stems as a whole as pi * (d/2)^2. This, however, might not be realistic as I think the basic water transporting unit is grass tiller, which has a much smaller cross sectional area compared to all the tillers as a whole trunk.

2. We can add a user-defined tiller number (n) for grass PFT, and calculate cross sectional area for each tiller roughtly as pi * (d/2)^2/n. But I'm not sure how this will influence the fate of a grass PFT under dry condition. Does mortality apply to the entire plant when one tiller failed (as they all have the same cross sectional area?) or there is a threshold percent of tiller hydo failure (e.g. 50% failure) and once passed we say the plant died (seems impossible as again they have exact same cross sectional area)?

Let me know which one sounds more reasonable or neither makes sense!

[rgknox]

Thanks for the response @XiulinGao. Judging by your agruments, it sounds like its not a simple solution, so lets move this over to a new issue. For the time being, we will accept this PR as is, but we will modify our testing so that the FATES hydro tests use the sapwood allometry mode 1 exclusively. How does that sound @glemieux and @XiulinGao ?

[glemieux]

@rgknox @XiulinGao that sounds like a good plan to me. I'll fire up an issue.

Update: see #1254

[glemieux]

Note for posterity: ESCOMP/CTSM@f2f7535 addresses the temporary hydro test workaround.

[rgknox]

Thanks for implementing that work around @glemieux