-
Notifications
You must be signed in to change notification settings - Fork 92
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
new and fixed parameters for nutrients, phenology, seeds, fire and age-mortality #595
Conversation
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.
Looks good. Thanks, @rgknox
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.
Grass phenology related updates look good to go @rgknox
Hi all, Is this sufficiently flexible? e.g., @rosiealice does this cover the PFT parameter needs for a FUN-like N fixation scheme? I'm imagining one PFT parameter would be a scalar variable that specifies in some units how much carbon a plant might allocate to N fixation, with zero meaning the plant isn't an N fixer. I could see this having a lower and upper bound to accommodate flexibility in representing obligate vs facultative N fixers, so therefore needing two PFT parameters? Other PFT parameters might be the N return on the C cost, or the constants controlling the temperature sensitivity of N fixation costs, though I would expect those to be global rather than PFT parameters, and if so, maybe we can add those in once we have a fully developed N fixation model? Does anyone see a need for more than 2 PFT-level N fixation parameters now as we build out the model? |
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.
One question, otherwise looks good. Sorry for the long delay in doing this!
…sed, these calls enforce that the parameter file has these names current.
updates: changed active_crown_fire to a scalar, and fixed the code comment noted by @pollybuotte |
@rgknox the active_crown_fire parameter will be an "on/off" switch to use the active crown fire portion of the code. It should be switched to a binary logical flag. 1=active crown fire 0=no active crown fire |
@jkshuman , how do those changes look? |
@rgknox looks good. Thanks. |
working with @JessicaNeedham to incorporate some of her new parameters as well, as some near-term integrations from her that use those parameters are forthcoming |
[Set of changes to follow with size- and age-dependent mortality modelled as increases in mortality rate with cohort size/age. Here just adding the parameters for these functions to the default param file. There is a rate of change and inflection point for size and age. Cohort age fusion parameter is also included - in age dependent mortality we track cohort age - cohorts cannot be fused if they are too different in age. ] Fixes: [NGT-ED Github issue #] User interface changes?: [Yes - users will have to specify these parameters] Code review: [Names] Test suite: [suite name, machine, compilers] Test baseline: Test namelist changes: Test answer changes: [bit for bit, roundoff, climate changing] Test summary: No testing.
merging Jessie's mort age parameters
@rgknox I am going to pull another parameter out of the fire code. CG_strikes is a scaler on lightning datasets to identify the percentage of cloud to ground strikes. Will be easier to test and modify this if it lives in the param file as a global scaler parameter. Will take care of it this week. |
All expected tests pass, b4b. namelist tests do not pass, as expected. /glade/scratch/rgknox/clmed-tests/fates-sci-1.31.1_api.8.1.0-Ceac01319-F26ab2875.fates.cheyenne.intel @jkshuman, can you take a look at the new cg_strikes parameter and sign off? |
I think my earlier comments to add size to a couple parameters got lost during the alphabetization step |
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
Co-Authored-By: Charlie Koven <[email protected]>
see my first question in the discussion of PR 590, here: #590 |
based on feedback so far on #590, I'm not going to add any changes related to hydraulics to this PR. I will go ahead and re-test this PR with @ckoven 's updates, this should be read to merge soon. |
all expected pass, with NLFAILs (expected fail). |
Ahh, I just missed this to add in a new parameter. My bad team. There is a fates_leaf level parameter needed for taking into account nutrients effects during photosynthesis. It's "flnr" for fraction of leaf N in Rubisco (gN Rubisco / gN leaf). |
sure, we are bound to have another round of parameter updates in the near future, we just just try to clump them whenever possible |
Description:
This set of changes focuses only on changes to the default parameter set.
Collaborators:
@qzhu-lbl, @evaleriksen, @pollybuotte, @slevisconsulting , @jkshuman, @glemieux , @xuchongang, @ckoven @rosiealice , @JessicaNeedham
Expectation of Answer Changes:
These changes should not change answers
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: