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

clean/refactor phenology #204

Closed
ckoven opened this issue Mar 29, 2017 · 2 comments · Fixed by #862
Closed

clean/refactor phenology #204

ckoven opened this issue Mar 29, 2017 · 2 comments · Fixed by #862

Comments

@ckoven
Copy link
Contributor

ckoven commented Mar 29, 2017

There are a couple issues with respect to phenology that need fixing. I'm just noting this now as a thing to do when we have time.

(1) most of the phenology parameters (effectively everything other than leaf_long, and the flags to use a given routine themselves) are not at the PFT level; thus we can't really have competition by PFT due to differing phenologic traits (other than overall strategy). perhaps not a big deal for NGT, but this ought to be fixed for all other biomes. simplest way is to just put the existing code within a PFT loop and calculate each of these site-level phenology switches separately for each PFT using possibly different parameters for each PFT

(2) there's at least one hard-coded PFT parameter that becomes a bug in the new PFT paradigm: https://github.com/NGEET/ed-clm/blob/master/components/clm/src/ED/biogeochem/EDPhysiologyMod.F90#L485
in which it assumes that the drought-deciduous PFT is number 7. This can (and ought to, at some point soon) be fixed by the above strategy...

@ckoven
Copy link
Contributor Author

ckoven commented Mar 30, 2017

I just realized that this is just a rediscovery of #65.

bandre-ucar added a commit that referenced this issue Apr 7, 2017
Merge branch 'newpftparams'

Pulled a bunch of PFT parameters, including the ones from @xuchongang
and @elias-massoud's sensitivity study, as well as others, into the
new FATES parameter file.

Parameters pulled out include both PFT-level parameters and scalar
parameters.

PFT parameters include:

 * allometric: for DBH:Bleaf, DBH:height, DBH:biomass, SAI
 * background mortality rates
 * soil moisture threshold for drought mortality in non-hydrodynamic model
 * kinetic parameters for vcmax, jmax, and tpu
 * seed turnover times for germination and decay (and thus, by ratio,
   seed-recruit survival fraction)

Scalar parameters include:

 * phenology. in the long run these should all be PFT-level, but
   punting on that for now (#204)
 * patch/chort fusion tolerances
 * CWD cellulose/lignin fractions
 * bb_opt for C3 and C4 plants
 * base respiration rates for Ryan respiration scheme

I also cleaned up the new FATES parameter file, to get rid of all the
CLM parameters that were still on it. So at this point only FATES
variables should be left there.

Fixes: #56, #75, #115, partial #65

User interface changes?: Yes, requires a new fates parameter file with
additional parameters.

Code review: requesting @xuchongang, @rosiealice, and @rgknox take a
look, and also the @rgknox run the rapid science check, since non-b4b.

Reasons for this being non b4b are because:

 1. moving the parameters from fortran to netcdf isn't expected to be
    b4b.

 2. There's a minor inconsistency, the old code had a number (1.4976)
    hard-coded in there, which wasn't quite correct for (10.0_r8**c)*m
    given the default parameters, which resolves to
    1.50030644180475. this code here was copied from @xuchongang and
    @elias-massoud's code, and seems correct, but wouldn't mind
    someone else checking to make sure this is as its meant to be.

 3. The high-temp scaling factors were specified once, but are now
    calculated inline to make consistent with kinetic parameters, this
    is the same to like 7 or 8 digits but not b4b.

Testing:
  ckoven:
    Test suite: ed suite, lawrencium, intel
    Test baseline: bc571f9
    Test namelist changes: new default parameter file
    Test answer changes: should be roundoff or close to it
    Test summary: functionality passes, not bit for bit as expected

  andre:
    Test suite: ed - yellowstone gnu, intel, pgi
                     hobart nag
    Test baseline: 2c82568
    Test namelist changes: new default parameter file
    Test answer changes: not bit for bit
    Test summary: all functionality tests pass. Not bit for bit as expected

    Test suite: clm_short - yellowstone gnu, intel, pgi
    Test baseline: clm4_5_12_r195
    Test namelist changes: none
    Test answer changes: bit for bit
    Test summary: all tests pass, bit for bit.
@rgknox
Copy link
Contributor

rgknox commented May 19, 2022

this will be addressed in #862

@rgknox rgknox mentioned this issue May 19, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants