-
Notifications
You must be signed in to change notification settings - Fork 94
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
Parteh scaling fixes #716
Conversation
…ition cases (prescribed N or P).
…aximum storage, and fixes to defining plant need and its relationship to storage
…TES-ECA, will be keyed off storage
Hey @rgknox nice job. |
@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. |
Using the latest That said there are two tests that have different values: |
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 |
…rom nutrient based 2d arrays
…suppress overshooting. Adjust logistic function to be only 1 parameter (k)
…CA uptake, 2) new storage fraction diagnostics, 3) function calls to get storage targets and 4) some code cleaning related to defining need
…Started initial work towards making history diagnostics decentralized
…lations and some updates to history
…e locations. Started shift to add in thaw depth to root depth calculations.
@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. |
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.
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 fullparteh
would aid readability imo. - Similarly (and this is probably not the right place for this comment),
scpf
represents size class by pft, why not usescpft
. Or, given that the iteration counter for pft isft
, for consistency why notscft
?
Thanks for the comments @walkeranthonyp |
…ded storage deficit history variables for canopy/understory
…hich prevents allocations when not needed
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. |
Tested this on cheyenne with modifications to check B4B. 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 |
Updated tests on cheyenne and izumi: ESCOMP/CTSM#1371 |
Description:
Coupled with: ESCOMP/CTSM#1371
This PR has a few fixes and updates to parteh and FATES CNP.
Collaborators:
@ckoven @qzhu-lbl @rosiealice @walkeranthonyp @glemieux @jenniferholm

Expectation of Answer Changes:
Checklist:
Test Results:
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: