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

Add Atkin et al 2017 leaf respiration model #931

Merged
merged 34 commits into from
Mar 7, 2023

Conversation

ckoven
Copy link
Contributor

@ckoven ckoven commented Nov 8, 2022

This code adds the option for a new leaf maintenance respiration model based on Atkin et al 2017.

Description:

We've been using the Ryan 1991 https://doi.org/10.2307/1941808 maintenance respiration model in FATES so far. Newer schemes have been developed that are based on a lot more data. Discussion in #729 pointed to the Atkin et al 2017 https://doi.org/10.1007/978-3-319-68703-2_6 as one that is based on a very large amount of observations. The model also suggests that respiration acclimates with growing temperature, with important implications (e.g. as explored in Huntingford et al https://doi.org/10.1038/s41467-017-01774-z)

fixes #729.

Some things to think about:

(1) Vertical N scaling and how it scales with respiration: In the Ryan 1991 model leaf respiration scales directly with leaf N, both at the leaf level and therefore also at the canopy level. Whereas in the Atkin et al 2017 model, respiration is not exactly proportional to leaf nitrogen -- there is a linear leaf N term but also an offset (due to the base PFT-dependent R_0 and growth-temperature-acclimation R_3 terms). So I think we need to understand how this governs the vertical profiles of respiration (and thus also the vertical profiles of leaf costs vs benefits that governs LAI).

(2) This code allows respiration, but not Vcmax, to acclimate to temperature. @alistairrogers makes the point that we probably want to shift to acclimating vcmax and respiration together. So possibly we may want to keep the old respiration model the default until we also have vcmax acclimation merged and also ready to use?

(3) This code only applies to leaves of C3 plants. Respiration from C4 plant leaves and all non-leaf organs is unchanged here.

(4) We may want to further encapsulate the C4 plant logic into its own subroutine for neatness. Also there are still some preexisting inline magic numbers in the old respiration code that could be better cleaned up and/or pulled out as parameters (Noted I think here).

(5) This code pulls out the R_0 terms as PFT parameters (which will need to be moved to a dedicated variable from their current spot in the EDPftvarcon_inst%dev_arbitrary_pft variable) but not the scalar parameters. We may want to pull all of them out into the parameter file?

(6) Overall I think a key question is whether we would want to make this the default leaf respiration model prior to / as part of the larger calibration exercise or not. This will tend to increase leaf respiration, which the model with current defaults is biased low on, but possibly require other more complex things to be worked out.
--Note that we have decided that this should be default-off for not, and perhaps updated as default-on via a subsequent PR.

Collaborators:

Discussion with @JessicaNeedham @jenniferholm @rgknox and also discussion in #729.

Expectation of Answer Changes:

This will change answers if activated. If we don't activate it, it should be bit for bit. Activating the new respiration model is via the parameter file flag fates_maintresp_model .

fixes #982, but because of that, it should no longer be bit-for-bit, even when deactivated.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
  • I have read the CONTRIBUTING document.
  • FATES PASS/FAIL regression tests were run
  • If answers were expected to change, evaluation was performed and provided

Test Results:

not yet run through test suite.

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

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

FATES baseline hash-tag:

Test Output:

@alistairrogers
Copy link

Hi Charlie, re #1 Just a reminder that Julien's work (currently in review) explored vertical gradients in N Rd and Vcmax. We found that the ratio between respiration rate and maximum carboxylation was smaller near the ground than at the top-of-canopy, and that the decrease in maximum carboxylation rate from the top-of-canopy to the ground was less than model assumptions. Vcmax - Na gradients matched as per model assumptions. This obviously implies that Na-Rd relationship would change with canopy depth.

@ckoven
Copy link
Contributor Author

ckoven commented Nov 8, 2022

Thanks @alistairrogers. Just wanted to note that the equations in Atkin et al 2017 will (I think) give the opposite sign to what you describe for the vertical profile in the ratio of Rd to Vcmax. Since the form of the equation is (R0 + R1 * N + R2 * Tgrowth), given the reported coefficients for R0 and R2, as long as Tgrowth is less than 37 Celsius, the sum of the first and third terms will be positive and so for low N/area as found deeper in the canopy, there will be relatively more respiration than N/area. That will hold for leaf respiration scaling within a given plant; between plants of different PFTs there could be other differences depending on their relative traits, and between plants of the same PFT in different canopy strata there could be differences based on the FATES logic that governs carbon starvation. But it isn't obvious that this scheme is consistent with the vertical profiles that Julien measures.

@alistairrogers
Copy link

Thanks doing laying that out @ckoven. I think the use of Atkin to resolve canopy gradient of Rd will lead to problems. Their approach should be sound for top of canopy Rd but I think will produce the opposite of what we would expect within a canopy. Respiration basically has two components the (1) supply - availability of substrate (photosynthate) to fuel respiratory processes which is lower closer to the ground (lower photosynthetic rates) and (2) the cost of maintaining existing function which is essentially driven by how much protein, primarily associated with photosynthesis, that you have. As we know leaf N and protein decrease as you get closer to the ground a potential third component (which would explain Julien's observations) would be respiration associated with repair from excess light (obviously higher at the top of the canopy). I find it hard to think of a scenario whereby photosynthetic capacity would be negatively correlated with Rd within a canopy. This is perhaps why the model assumption of Rd = Vcmax * 0.015 is so robust. Perhaps FATES could use Atkin to set top-of-canopy Rd but then scale within a canopy using either existing fixed ratios or new ratios based on Julien's work.

@rosiealice
Copy link
Contributor

I feel like there is a pretty interesting analysis to be done on the CUE of different respiration, nitrogen and vcmax scaling schemes. VDMs increasingly need to know this (at least, the ones that resolve layered physiology) but the available meta analyses don't afaik provide much guidance...

We could probably use SP mode for this, and just turn on the lmr.

But in the interim we do need some way of decreasing the CUE in FATES, and I think adding the Atkin option, if only for the purposes of testing it and finding that it kills everything, is worthwhile!

@@ -1020,7 +1020,8 @@ data:

fates_damage_recovery_scalar = 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ;

fates_dev_arbitrary_pft = _, _, _, _, _, _, _, _, _, _, _, _ ;
fates_dev_arbitrary_pft = 1.7560, 1.4995, 1.4995, 1.7560, 1.7560, 1.7560,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ckoven what would we call this parameter? I see it matches r_0: "! base respiration rate, PFT-dependent (umol CO2/m**2/s)"

Also, is there a similar parameter in the Ryan model that could also leverage the parameter?

I could add this parameter to the nutrient coupling PR (which has other parameter changes, and assuming it gets through queue more quickly).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, and good idea. Maybe fates_maintresp_atkinetal2017model_baserate? There isn't a direct analogue in the Ryan 1991 model.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about: fates_maintresp_atkin2017_baserate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgknox that works for me, thanks!

@@ -1490,7 +1491,7 @@ data:

fates_leaf_theta_cj_c4 = 0.999 ;

fates_maintresp_model = 1 ;
fates_maintresp_model = 2 ;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what is our staging for making the change in the default file? My sense from discussion with others is that the Atkin model is the path forward. However, all of the other parameters are calibrated around the Ryan model, and this will undoubtedly have an impact on output.

Was the plan to switch over to Atkins first, and accept the differences good or bad to output, and then continue with other calibrations? Or was the plan to switch over to Atkins during a more comprehensive calibration exercise perhaps with the satphen simuluation or other biogeographically constrained testbed?

In either situations, we should probably run this through the ilamb mill to get a sense of the changes.

Copy link
Contributor Author

@ckoven ckoven Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for staging purposes, we should bring in the code first with this switch reverted to the current value, and if we subsequently decide to make it the default model, that would likely need to be accompanied by other default parameter changes as well.

@ckoven
Copy link
Contributor Author

ckoven commented Nov 9, 2022

@alistairrogers That is an interesting idea, to use the published equation for relating leaf N/area to respiration rates only to the top of canopy value and then assuming proportionality with N/area vertically through the canopy. Worth exploring the dynamics of that at least, and seeing whether the evidence available is generally supportive of it. Per @rosiealice's comment, I'm not sure if the available data speaks to it either way, but agreed that we should investigate.

@ckoven
Copy link
Contributor Author

ckoven commented Jan 12, 2023

I want to raise a question here. As best I can tell, none of the Atkin papers really cover C4 plants, since they are not well represented in the GlobResp database that that parameterization is based on. I had thought that CLM5 defaulted entirely to the old model for C4 plants, but I just noticed here in the tech note and here in the code, that it is actually using the same Atkin et al 2015 algorithm and traits for C4 grasses as for C3 grasses for the base respiration rate (which acclimates based on 10-day temperature), but then a different short-term temperature scaling for C3 and C4 plants where the C4 plants follow the old model. JULES does not use the Atkin scheme for C4 plants, at least as reported here in the Huntingford et al 2017 paper. Big-leaf ELM still uses the pre-CLM5 scheme for all PFTs (which includes the different temperature scaling for C4 plants). I don't know what the best solution here is, so figured I'd at least log this here and open for discussion if anyone knows the best thing to do.

@dlawrenncar
Copy link

dlawrenncar commented Jan 12, 2023 via email

@jkshuman
Copy link
Contributor

@danicalombardozzi any thoughts or recommendations on the C4 conversation here? Or recommendations for moving forward.

…lue of fates_base_mr_20 in fates_params_default.cdl, and updating number in fates_params_default.cdl
This commit creates two xml patch files to facilitate
this update.  The file names are to be applied in the
order denoted by the number suffix on the xml file name.

The first file is used to create the new Atkin parameter
as well as update the mortality rate typo.  The UpdateAPI.py
tool is modified to allow for variables to be added that
are undefined.

The second file updates the new Atkin parameter using
BatchPatchParams.py.  The update intentionally leaves
the final pft as undefined.

Modifying and creating new variables with mixed values,
in this case leaving one undefined, is what necessitated
the update to the tooling and the sequential tool calls.
The call to update the global history attribute of the
output file was missing a further call to the sort tool.
This was fixed by moving the history update to earlier
in the code.

Note there is still an issue in which the resulting
parameter file cdl results in an incorrect name due
to the use of chained temp files in the tool calls.
@glemieux
Copy link
Contributor

Per @ckoven I tested this pr at 22212b5 to make sure that this would run in all the modes we test knowing there would be diffs. All expected tests ran successfully.

Folder location on cheyenne: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr931-atkin-on

@glemieux
Copy link
Contributor

glemieux commented Feb 25, 2023

@rgknox I updated the default parameter file to revert the changes to the fates_dev_arbitrary_pft variable and added the new respiration parameter. I created two patch files and updated the tooling to do this as the new parameter has an mixed values which the tooling couldn't quite handle. I also fixed the BatchPatchParameter.py tool to make sure the sorting was the last thing being called as we discussed.

I've got final regression tests against the current baseline going on cheyenne now.

@glemieux
Copy link
Contributor

All expected tests pass on cheyenne. The tests are not b4b as expected due to the bug fix in 10db8b9 that is now using the appropriate parameter variable instead of the hardcoded value. It should be noted that the parameter file will be updated with the correction to this variable in the HLMs at a later date so the baselines will have non-b4b differences during that update as well.

Location on cheyenne: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr931-final

@glemieux
Copy link
Contributor

glemieux commented Feb 27, 2023

Per the software meeting today, it was decided that @ckoven would test out using the C3 Atkin parameter for the C4 grasses. If this doesn't break the model then we will make the update to the parameter file and the parameter xml patch file. It was also decided to update the parameter file to utilize a new agnostic naming scheme for the related variables per @rgknox's slides here: https://docs.google.com/presentation/d/1tnttlh9CGFG5fTRjyZf-ALXMKyVOF8OnFnQqmTyJmJk/edit

@ckoven
Copy link
Contributor Author

ckoven commented Mar 1, 2023

I've updated this to reflect the new decision on what to do about C4 grasses, which is now also in the code for this PR. The decision is that it doesn't really make sense to use an entirely different leaf maintenance respiration model for C3 and C4 plants, particularly given the large difference in baseline maintenance respiration rates between the two models, which could undermine attempts to meaningfully compete C3 and C4 grass PFTs. So, in the absence of better options, we decided to use the same base respiration rates and temperature sensitivities from C3 grasses for the C4 grasses as well, until such time as better data is available to parameterize the C4s separately. Note that this is also what CLM5 does. Choosing the Ryan 1991 model reverts to the same behavior as before (where C4 grasses have the same basal respiration rates per unit leaf nitrogen but a different temperature sensitivity than the C3 PFTs).

Per precedent, the patch file api number should be
the api number of the file to which it is being
applied.
@glemieux
Copy link
Contributor

glemieux commented Mar 7, 2023

Regression tests on Cheyenne are complete. All expected tests pass RUN and all result in DIFFs due to the fix mentioned above.

Folder location: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr931-c4update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants