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

New allometric modes #1128

Merged
merged 21 commits into from
Apr 23, 2024
Merged

New allometric modes #1128

merged 21 commits into from
Apr 23, 2024

Conversation

mpaiao
Copy link
Contributor

@mpaiao mpaiao commented Dec 8, 2023

Description:

  1. New allometric function for above-ground biomass (allom_amode = 4), which is similar to Chave et al. (2014), except that the exponent for wood density is independent of the one for "tree size" (i.e., DBHDBHHeight). Although this function could be simplified externally (by incorporating wood density into the scaler), this makes the dependency on wood density more transparent and may make parameter perturbation experiments a bit easier.
  2. New allometric function for leaf biomass and crown area (allom_lmode = 4), which uses tree size as the predictor. In the case of leaf biomass, the function also is a function of top-canopy SLA, using a similar formulation as the one for above-ground biomass.
  3. Crown depth has now allometric modes too (variable allom_dmode). If allom_dmode=1, crown depth is a constant fraction of height (the former default); when allom_dmode=2, crown depth is defined as p1 * Height^p2, following Poorter et al. (2006). Former parameter crown_depth_frac was replaced with parameter allom_h2cd1, and a new parameter allom_h2cd2 was added for the exponent.
  4. All allometric mode variables were converted to integers when they are loaded (parteh/PRTParamsFATESMod.F90). This avoids converting them to integers every time the allometric function is called.
  5. Small edits to CheckParams (parteh/PRTParamsFATESMod.F90).
  • The current implementation of allom_hmode=2 is a bit unusual because the minus sign of the Weibull function is incorporated in parameter p2 (meaning that p2 must be negative). Added a check to make sure the parameter is negative.
  • Instead of aborting the run as soon as an inconsistency in parameters is spotted, the subroutine now continues to check other parameters, and report all the errors before stopping the run. This avoids users recompiling/submitting FATES multiple times to fix one inconsistency at a time.

Collaborators:

Expectation of Answer Changes:

Checklist

If this is your first time contributing, please read the CONTRIBUTING document.

All checklist items must be checked to enable merging this pull request:

Contributor

  • The in-code documentation has been updated with descriptive comments
  • The documentation has been assessed to determine if updates are necessary

Integrator

  • FATES PASS/FAIL regression tests were run
  • Evaluation of test results for answer changes was performed and results provided

Documentation

Test Results:

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

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

FATES baseline hash-tag:

Test Output:

…provements.

1. New allometric function for above-ground biomass (allom_amode = 4), which is
   similar to Chave et al. (2014), except that the exponent for wood density is
   independent of the one for "tree size" (i.e., DBH*DBH*Height).
2. New allometric function for leaf biomass and crown area (allom_lmode = 4),
   which uses tree size as the predictor. In the case of leaf biomass, the
   function also is a function of top-canopy SLA, using a similar formulation
   as the one for above-ground biomass.
3. Crown depth has now allometric modes too (variable allom_dmode). If
   allom_dmode=1, crown depth is a constant fraction of height; when
   allom_dmode=2, crown depth is defined as p1 * Height^p2. Former parameter
   crown_depth_frac was replaced with parameter allom_h2cd1 (p1), a new
   parameter allom_h2cd2 was added for the exponent.
4. All allometric mode variables were converted to integers when they are loaded.
   This avoids converting them to integers every time the allometric function is
   called.
5. Small edit to CheckParams. The current implementation of allom_hmode=2 is a
   bit unusual because the minus sign of the Weibull function is incorporated
   in parameter p2 (meaning that p2 must be negative). Also, instead of aborting
   the run as soon as an inconsistency in parameters is spotted, the subroutine
   now continues to check other parameters, and report all the errors before
   stopping the run. This avoids users submitting FATES multiple times to get
   one inconsistency at a time.
@glemieux glemieux added parameter file Pertaining to changes to the FATES parameter file enhancement labels Dec 8, 2023
@rgknox
Copy link
Contributor

rgknox commented Jan 15, 2024

awesome set of updates @mpaiao looking forward to getting this integrated

@rgknox rgknox self-assigned this Jan 15, 2024
@rgknox
Copy link
Contributor

rgknox commented Jan 15, 2024

I was looking through an old issue related to fates Hydro and how it perceives and uses crown depth, see here: #674

I think the things we discuss in that thread are out of scope for this PR, but would be good to re-evaluate and move forward on them though.

@rgknox
Copy link
Contributor

rgknox commented Jan 15, 2024

Just so people are aware, we don't call the crown depth allometry module in hydro:
https://github.com/NGEET/fates/blob/sci.1.70.0_api.32.0.0/biogeophys/FatesPlantHydraulicsMod.F90#L775-L776

That crown depth routine is really only relevant for fire dynamics, as the radiative transfer doesn't use crown depth in its calculations.

@@ -1616,12 +1625,10 @@ subroutine leaf_area_profile( currentSite )
! is obscured by snow.

layer_top_height = currentCohort%height - &
( real(iv-1,r8)/currentCohort%NV * currentCohort%height * &
prt_params%crown_depth_frac(currentCohort%pft) )
( real(iv-1,r8)/currentCohort%NV * crown_depth )
Copy link
Contributor

Choose a reason for hiding this comment

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

nice update here @mpaiao

This will have a conflict with the new two-stream PR. I created a new subroutine that calculates the leaf area of crown layers, and it will need to leverage your fixes, see: https://github.com/NGEET/fates/pull/1141/files#diff-4fd8508e4a25680b45ecbfec6b6d80980a5b1e358af731203ee9d29266be46dbR2655

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for spotting this, @rgknox. Once the two-stream PR is integrated, I can update this part. I will leave this conversation open so we don't forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rgknox Should I go ahead and edit subroutine VegAreaLayer now that two-stream has been merged?

@@ -305,7 +306,7 @@ subroutine h2d_allom(h,ipft,d,dddh)
p3 => prt_params%allom_d2h3(ipft), &
allom_hmode => prt_params%allom_hmode(ipft))

select case(int(allom_hmode))
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, smarter to convert the switch on the read in, no need to convert over and over again in the dynamics loop

@@ -522,6 +534,12 @@ subroutine carea_allom(dbh,nplant,site_spread,ipft,crowndamage,c_area,inverse)
call carea_2pwr(dbh_eff,site_spread,d2bl_p2,d2bl_ediff,d2ca_min,d2ca_max, &
crowndamage, c_area, do_inverse)
capped_allom = .true.
case (4)
dbh_eff = min(dbh,dbh_maxh)
Copy link
Contributor

Choose a reason for hiding this comment

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

Noting that height capping is compatible with this scheme and implementation makes sense.

!--- Local variables
real(r8) :: duse
!---~---

Copy link
Contributor

Choose a reason for hiding this comment

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

header and descriptions are nice and verbose, units, refs, great

return
end subroutine size2dbh
! ============================================================================

Copy link
Contributor

Choose a reason for hiding this comment

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

You were very determined to find the inverse solution for dbh and h @mpaiao , not easy, kudos

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is totally borrowed from ED2 solvers, I just adapted it here. This allometric function is well behaved and should quickly converge with the Newton method, but I thought I should keep the full root finding approach in case we want to adapt for less well-behaved functions anywhere in FATES in the future.

write(fates_log(),*) '---~---'
write(fates_log(),*) ''
write(fates_log(),*) ''
nerror = nerror + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Like this

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree.

if (nerror > 0) then
write(fates_log(),*) 'One or more parameter errors found. Aborting.'
call endrun(msg=errMsg(sourcefile, __LINE__))
end if
Copy link
Contributor

Choose a reason for hiding this comment

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

double checked to make sure that all checks removed endrun and added nerror, looks good

@rgknox
Copy link
Contributor

rgknox commented Jan 15, 2024

@mpaiao these changes look pretty clean to me. The root finding method is complicated, and I did not go through it in detail. But I did see that it was borrowed from ed2? If so, that is great, since its already been vetted. After discussion, I realized that the 2 is a derivative of an x^2. There is no precedent of explaining that in the other methods, so I have no further comments. Approvinng

Copy link
Contributor

@JessicaNeedham JessicaNeedham left a comment

Choose a reason for hiding this comment

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

Thanks @mpaiao - these are great changes/additions. I was wondering whether we want to put suggested parameter values for various allometry options, since default parameters will be wacky with some of the options. We already do this for the Chave d2h and dh2bagw but we could add it in for some others as well. Maybe out of scope for this PR, but might be helpful in future.

biogeochem/FatesAllometryMod.F90 Outdated Show resolved Hide resolved
! degradation on water, energy, and carbon cycling of the Amazon
! tropical forests. J. Geophys. Res.-Biogeosci. 125:
! e2020JG005677. doi:10.1029/2020JG005677.
!
Copy link
Contributor

Choose a reason for hiding this comment

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

The new leaf biomass allometry looks great to me. One minor suggestion is to put suggested parameter values. It looks like the default d2bl1, d2bl2 and d2bl3 will give very high biomass with this equation so some suggested values for users wishing to use this option could be helpful.

biogeochem/FatesAllometryMod.F90 Outdated Show resolved Hide resolved
write(fates_log(),*) '---~---'
write(fates_log(),*) ''
write(fates_log(),*) ''
nerror = nerror + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree.

@rgknox rgknox closed this Apr 1, 2024
@rgknox rgknox reopened this Apr 1, 2024
@glemieux glemieux self-assigned this Apr 9, 2024
Bug fix. Adrianna Foster identified that trunks should be removed from the patch-level variable sum_fuel before being used to calculate rate of spread and fuel consumption.
Fixes exact restart bug for satellite phenology mode
@glemieux
Copy link
Contributor

glemieux commented Apr 23, 2024

I re-ran the fates regression tests on derecho since it had been a while since the last tests Ryan ran. As expected, all expected tests passed b4b.

Location: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr1128-fates-2

@glemieux glemieux merged commit adfa664 into NGEET:main Apr 23, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement parameter file Pertaining to changes to the FATES parameter file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants