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

Parteh scaling fixes #716

Merged
merged 35 commits into from
May 26, 2021
Merged

Parteh scaling fixes #716

merged 35 commits into from
May 26, 2021

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented Dec 8, 2020

Description:

Coupled with: ESCOMP/CTSM#1371

This PR has a few fixes and updates to parteh and FATES CNP.

  1. There are fixes to how prescribed nutrient uptake is applied, both in its scaling and switching on the flux of nutrients to the HLM as litter
  2. FATES is responsible for calculating the decomposer biomass in the competitive nutrient environment in the soil. This sounds counter intuitive, but this is so because the parameter that controls the decomposer biomass is associated with the PFTs inhabiting the soil, and thus FATES needs to calculate the decomposer biomass by weighting the amount of root biomass. The change added here is just a fix, it was being calculated wrongly, and now it is applied with a depth attenuation similar to what @qzhu-lbl has been using
  3. a new "cn_scalar" option was added. cn_scalar acts as a plant "neediness" function, which decreases as the plant has nutrients and increases when the plant is low. This scalar then operates on the plant demand that is used in the ECA MM equations for competitive uptake. This new option defines cn_scalar with a logistic function, the parameters which are defined locally as named constants. The curve is shown below.
  4. Nutrient storage, and particularly, the maximum nutrient storage has been re-defined for CNP runs. Previously, nutrient storage was tied to carbon storage with some stoichiometry target. Now, nutrient storage is assumed to account for some fraction of the total nutrient bound in living tissues (leaf, fnrt, sapwood). The stoichiometry parameter in the fates parameter file for storage now defines that fraction, instead of a stoichiometry ratio (like CN or CP).
  5. Coupling to enable CLM/ELM methane model to run concurrently

Collaborators:

@ckoven @qzhu-lbl @rosiealice @walkeranthonyp @glemieux @jenniferholm
cn_scalar_logi_k

Expectation of Answer Changes:

Checklist:

  • 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:

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

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

FATES baseline hash-tag:

Test Output:

@rosiealice
Copy link
Contributor

Hey @rgknox nice job.
I am wondering about whether there is any way of imposing a cost to the plant of having a higher competitiveness for N when the demand is high? In principle being more competitive with the microbes should require some kind of resource? Could one impose a respiratory cost on it?

@rgknox
Copy link
Contributor Author

rgknox commented Dec 9, 2020

@rosiealice , I'm not sure on what is an acceptable solution for the time being. I agree that having no cost associated with cn_scalar seems off. My plans were to use this formulation for the short term, as a way of simply preventing the plants from excessive uptake to the point they would just be spilling it back out as efflux. My medium term plans were to soon implement a tendency (much like how we do trimming) to the fine-root target carbon pool that is responsive to the nutrient storage pool, this of course would implicitly have a cost associated with it. With this option, the cn_scalar term would then revert to a constant and be calibrated in with the vmax term.

@glemieux glemieux self-assigned this Dec 14, 2020
@glemieux
Copy link
Contributor

glemieux commented Dec 23, 2020

Using the latest fates_main_api and pre-merging in the current fates tag the regression tests are passing although not b4b. All tests have an expected difference in the change to the nclmax dimension size in EDTypesMod.

That said there are two tests that have different values: ERS_D_Ld30.f45_f45_mg37.I2000Clm50FatesCruGs.cheyenne_intel.clm-FatesPRT2 and SMS_Lm13.1x1_brazil.I2000Clm50FatesCruGs.cheyenne_gnu.clm-FatesColdDef. The former makes sense since the test sets fates_parteh_mode = 2 engaging CNP. I'm not sure why the 1x1_brazil test would reveal difference for the gnu compiler version, but not the intel version, however. Currently investigating.

@rgknox
Copy link
Contributor Author

rgknox commented Dec 31, 2020

I will revert nclmax to 2. If we decide to use more than 2 canopy layers by default, it should be its own PR. Sound good?

@glemieux
Copy link
Contributor

glemieux commented Jan 4, 2021

I will revert nclmax to 2. If we decide to use more than 2 canopy layers by default, it should be its own PR. Sound good?

Sounds good

@glemieux glemieux added the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Jan 11, 2021
@rgknox
Copy link
Contributor Author

rgknox commented Apr 11, 2021

The following output is from a comparison of 3 different model configurations at BCI. They all use the same inventory initialization, all simulations are for 20 years using the same looping met forcing on site.
"master:" uses code from master as of tag: sci.1.43.4_api.14.2.0, and uses the carbon-only hypothesis
prt2-new-order: uses the code from this branch, includes N & P cycling but with synthetic uptake meeting 100% of need, and assumes that leaves have sole priority to replace turnover loss
prt2-old-order: uses the code from this branch, includes N & P cycling but with synthetic uptake meeting 100% of need, and assumes the same allocation prioritization that we have in the carbon-only model (ie leaves AND fine-roots get equal priority to replace turnover losses)
fates_bci_tseries_prt1_vs_prt2_oldneworder

The key point is that when we try to reproduce the resource priority order of carbon-only algorithm, as well as provide unlimited uptake to meet demand, the C+N+P version of the code gives roughly the same results. This is good, and what I was hoping to see!

The second key point is that allocation priority has a very large impact on results. However, note that there is no trade-off for not allocating to root. Having more allocation to root does not impact the plants ability to uptake water, as with hydro turned off, it really does not affect btran as long as there is water available in the soil. Also, since this is not fully coupled with below-ground nutrient cycling, root allocation does not impact nutrient uptake. Whereas, if this simulation was fully coupled with the FATES-ECA formulation, plants without fine-roots would be at a disadvantage competing for mineralized N and P. So paying the respiration cost of fine roots just makes a plant un-competitive in this model formulation.

@walkeranthonyp
Copy link

@rgknox this is looking good. Agreed on your analysis of the allocation priority. Roots are solely a cost right now, when they have an associated benefit the character of the results will change. Taking a look at the code now.

Copy link

@walkeranthonyp walkeranthonyp left a comment

Choose a reason for hiding this comment

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

Just leaving a comment as I don't feel fully qualified yet to really approve anything. But on the whole all looks good. I'm supportive of the decoupling of nutrient storage from carbon storage and accounting for stored nutrients within the overall stoichiometry of each organ.

The cn_scalar for ECA sounds good, though is it really necessary given a cost is not associated with it? Agreed with @rosiealice that a cost for this would be appropriate, though also agreed that there's no clear solution for this. The root Vmax is equivalent in many ways to leaf Vcmax. It could be linked to tissue N in some way and that would have a respiratory cost. The cn_scalar equivalent in leaves would be a reduction in Vcmax in response to lower demand/supply. Short term this might occur through deactivating RuBisCO (which presumably has lower resp costs) and longer term through lower leaf N requirements. Given we don't do this for leaves, should be do it for roots? A balanced solution might be to make root vmax a function of root N. Though again, not necessary now but definitely food for future development if we want an integrated approach to resource allocation and acquisition.

One additional general comment and a couple of potentially annoying comments (feel free to take no action on any of them).

  • What's the logic for removing reproductive organs from the default parteh mode?
  • I noticed for variable naming etc PARTEH is represented by prt. Given this is only saving three characters is the abbreviation necessary? Using the full parteh would aid readability imo.
  • Similarly (and this is probably not the right place for this comment), scpf represents size class by pft, why not use scpft. Or, given that the iteration counter for pft is ft, for consistency why not scft?

@rgknox
Copy link
Contributor Author

rgknox commented Apr 20, 2021

Thanks for the comments @walkeranthonyp
Reproductive allocation should still be active, and follow the same hypothesis for both c-only and cnp. In both the c-only and the cnp hypothesis, reproducton is currently relegated to be a fraction of allocation during stature growth. Since in both cases, we don't allocate to it before stature growth (during the phase that replaces turnover losses), and since reproductive tissues are immediately shed from the plant post allocation, it has a special status. Because of this special status, its not included in the priority ordering.
I think your last point make a lot of sense. I think your second point makes sense, but indeed, would require a lot of code changes, and would be a more painful correction.

@rgknox rgknox removed the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label May 5, 2021
@rgknox
Copy link
Contributor Author

rgknox commented May 5, 2021

Update: Added coupling between FATES and the methane model. Coupling with methane is important for calculating oxic/anoxic content of soil which is subsequently necessary for calculating denitrification and nitrification rates.

Removing the Not Ready label.

@rgknox
Copy link
Contributor Author

rgknox commented May 10, 2021

Tested this on cheyenne with modifications to check B4B.
/glade/scratch/rgknox/clmed-tests/parteh-scaling-fixes-api-C7bd490c2-Fabb43f0b-test-b4b-v4.fates.cheyenne
Tests are as expected. Tests are b4b with master with the following expected exceptions:
The tests with LITTER_OUT should have differences as there is a fix to how litter fluxes are area scaled as diagnostics, to include seed fluxes
PRT2 tests should have different results as that is the main conceptual target of this PR

Here are the sub-branches to this PR that have the non-b4b changes suspended:

https://github.com/rgknox/fates/tree/parteh-scaling-fixes-may05-b4b
https://github.com/rgknox/ctsm/tree/fates_parteh_scale_fixes_api_b4b

@rgknox
Copy link
Contributor Author

rgknox commented May 26, 2021

Updated tests on cheyenne and izumi: ESCOMP/CTSM#1371

@glemieux glemieux added tag: next Next PR to be incorporated ctsm An issue is related to ctsm host land model or a particular PR has a corresponding ctsm-side PR labels May 26, 2021
@rgknox rgknox merged commit 1723d14 into NGEET:master May 26, 2021
@rgknox rgknox deleted the parteh-scaling-fixes branch October 31, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ctsm An issue is related to ctsm host land model or a particular PR has a corresponding ctsm-side PR tag: next Next PR to be incorporated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants