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

parameter file updates #862

Merged
merged 35 commits into from
Jun 21, 2022
Merged

parameter file updates #862

merged 35 commits into from
Jun 21, 2022

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented Apr 27, 2022

Description:

This set of changes brings in a number of new parameter constants to the fates parameter file, along with their model data structures and use. This set should include a number of new parameters from different initiatives including, phenology, crown damage, nutrient cycling, hydraulics, etc. This set of changes will also add the maximum number of patches (and potentially the number of cohorts per patch) to the parameter file workflow, which will be used to drive the CLM/ELM side patch allocations when fates is on, and will involve some re-organization of the fates initialization call sequence.

Since this is already and API changing PR, this may also include a call to finalize and declare the tallies of un-reported message counts in the log (for those messages that saturated).

Fixes: #799
Fixes: #204
Fixes: #214
Fixes: #334

Synchronized with: ESCOMP/CTSM#1766

Collaborators:

see issue #799

Expectation of Answer Changes:

At this point, no answer changes are expected.

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:

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

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

FATES baseline hash-tag:

Test Output:

@rgknox rgknox added the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Apr 27, 2022
@glemieux glemieux added this to To do in Pull Request Prioritization and Status via automation May 2, 2022
@rgknox
Copy link
Contributor Author

rgknox commented May 13, 2022

Update on this PR:

I've created a script that will allow a user to automatically update an existing parameter file (cdl) to a newer format, based on an xml file. This script and xml couplet will define parameter name changes, parameter deletions, parameter attribute and value changes, dimension name changes and dimension deletions.

xml as of 7912585: https://github.com/NGEET/fates/blob/7912585bb006b514a0ee97cf8de7e829a63ce17f/parameter_files/apichange_23to24.xml

script as of 7912585: https://github.com/NGEET/fates/blob/7912585bb006b514a0ee97cf8de7e829a63ce17f/tools/UpdateParamAPI.py

This script allows us to apply significant updates to the parameter file, while also ensuring that users who have personal parameter files, don't get left behind. They can also use this script/xml couplet to update their files.

After consulting with the FATES software team, we agreed it would be useful to re-organize the parmeter names. In particular, add more group names to parameter name prefixes. All changes to parmeter name changes are summarized in this table, and are subject to updates and feedback from the broader FATES team

https://docs.google.com/spreadsheets/d/1ZdhM8Ggv6wlqFNKdTvtWRMGHdb_7ae1zuIVfl1RKQHw/edit?usp=sharing

Summary of changes:
Added specific changes associate with #799
Removed the "prt" group (which was opaque) and converged those parameters into an alloc, turnover and stoich group
Converged parameters into the following groups, when possible:

  • "mort" for mortality.
  • "phen" for phenology
  • "alloc" for allocation
  • "turnover" for turnover
  • "stoich" for stoichiometry
  • "cnp" for all things using dynamic Nitrogen or Phosphorus
  • "lu" for all things, including logging, that are landuse related
  • "leaf" for all things, including photosynthesis, that are leaf related
  • "nonhydro" for parameters that only used when hydro is off
  • "hydro" (previously "hydr") for all parameters that are only used when hydro is on
  • "allom" (which existed but was slightly expanded) for allometry
  • "fire" already existed and was not changed much
  • "damage" for parameters only active when the damage module is on
  • "frag" for litter fragmentation
  • "recruit" for all things related to recruitment, including on-tree allocation to seeds and seed germination
  • "rad" for all things radiation and optical related

@glemieux glemieux moved this from To do to Not Ready/WIP in Pull Request Prioritization and Status May 16, 2022
@glemieux
Copy link
Contributor

Per fates software meeting discussion (May 16):

  • lu to landuse
  • Better name than nonhydro?

@rgknox rgknox added PR status: Needs Review parameter file Pertaining to changes to the FATES parameter file and removed PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. labels May 31, 2022
@glemieux glemieux moved this from Not Ready/WIP to To do in Pull Request Prioritization and Status May 31, 2022
@rgknox rgknox requested a review from glemieux June 2, 2022 16:23
@glemieux glemieux moved this from To do to Under Review in Pull Request Prioritization and Status Jun 6, 2022
Copy link
Contributor

@glemieux glemieux left a comment

Choose a reason for hiding this comment

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

A few questions and clarifications.

biogeochem/EDCanopyStructureMod.F90 Outdated Show resolved Hide resolved
biogeochem/EDCanopyStructureMod.F90 Outdated Show resolved Hide resolved
@@ -46,9 +44,7 @@ module PRTParametersMod
! !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

real(r8), allocatable :: nitr_stoich_p1(:,:) ! Parameter 1 for nitrogen stoichiometry (pft x organ)
real(r8), allocatable :: nitr_stoich_p2(:,:) ! Parameter 2 for nitrogen stoichiometry (pft x organ)
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find this in the issue discussion. What predicated the removal of these parameters?

Copy link
Contributor Author

@rgknox rgknox Jun 8, 2022

Choose a reason for hiding this comment

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

We don't have a defensible way of having plasticity in nutrient concentrations yet, so this parameter is not useful. @jenniferholm has been investigating such a feature though, so this may change down the line. In the future depending on those findings we could introduce the necessary parameters. My sense is that when we do, the plasticity will not be introduced through this parameter, and may target individual organs that actually experience plasticity, like leaves.

parteh/PRTLossFluxesMod.F90 Show resolved Hide resolved
main/FatesHydraulicsMemMod.F90 Show resolved Hide resolved
Pull Request Prioritization and Status automation moved this from Under Review to Approved/Final Testing Jun 7, 2022
@rgknox rgknox merged commit 32bbd14 into NGEET:master Jun 21, 2022
@jenniferholm
Copy link
Contributor

😁 👍

@rgknox rgknox deleted the params-maxpatch-logfinal branch October 31, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parameter file Pertaining to changes to the FATES parameter file
Projects
No open projects
3 participants