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

Crown damage module #788

Merged
merged 104 commits into from
Oct 31, 2022

Conversation

JessicaNeedham
Copy link
Contributor

@JessicaNeedham JessicaNeedham commented Oct 1, 2021

Description:

This set of changes introduces a crown damage module into FATES. This reduces the crown area and biomass of tissues in the crown -leaves, sapwood, structure, storage and reproductive tissues. Damage can affect canopy or understory cohorts (or both) using the switches hlm_use_canopy_damage and hlm_use_understory_damage. Cohorts are split into damage classes, the number of which is set via the parameter ncrowndamage. The parameter damage_frac sets the fraction of each cohort that gets damaged in each damage event (currently hard coded to be once per year - but working on making this more flexible). The fates_damage_recovery_scalar parameter sets the degree of recovery i.e. allocation to regrow the crown versus staying on damaged allometric trajectory. Two optional parameters can be used to introduce damage-dependent mortality, which captures mortality from damage via mechanisms not represented in FATES. With default parameterisation this would be off.

Collaborators:

@ckoven @glemieux @rgknox @rosiealice

Expectation of Answer Changes:

With the hlm_use_canopy_damage and hlm_use_understory_damage off runs should be bit for bit with master. Currently that is not the case and I'm looking into why.
With these switches on then answers will not be bit for bit with master. There will be more cohorts as damage and recovery involve splitting cohorts into damage classes. Damaged cohorts will have smaller canopies which affects AGB and canopy dynamics. Depending on parameterisation carbon starvation mortality may increase.

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:

JessicaNeedham and others added 5 commits September 29, 2021 16:27
[one of the checks in EDPftvarcon is breaking runs with
1 pft. Cancel out the endrun message as a quick fix.  ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ This check is not related to damage work and is being
addressed in other issues/PRs ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[  ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
…ge_module' into JessicaNeedham-crowndamage_module
@JessicaNeedham JessicaNeedham marked this pull request as ready for review October 1, 2021 20:46
@glemieux glemieux added the PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. label Oct 1, 2021
Copy link
Contributor

@ckoven ckoven left a comment

Choose a reason for hiding this comment

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

@JessicaNeedham this is really great, thanks! I have some comments/questions, mostly just to improve clarity.

main/DamageMainMod.F90 Outdated Show resolved Hide resolved
main/EDTypesMod.F90 Outdated Show resolved Hide resolved
main/EDTypesMod.F90 Outdated Show resolved Hide resolved
@@ -166,15 +166,15 @@ module EDTypesMod

! COHORT TERMINATION

real(r8), parameter, public :: min_npm2 = 1.0E-7_r8 ! minimum cohort number density per m2 before termination
real(r8), parameter, public :: min_npm2 = 1.0E-12_r8 ! minimum cohort number density per m2 before termination
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to change here? is it a thing to bracket in if statement on the damage, or should we include even when no damage is running? seems like it will change answers if so.

real(r8), parameter, public :: min_patch_area = 0.01_r8 ! smallest allowable patch area before termination
real(r8), parameter, public :: min_patch_area_forced = 0.0001_r8 ! patch termination will not fuse the youngest patch
! if the area is less than min_patch_area.
! however, it is allowed to fuse the youngest patch
! if the fusion area is less than min_patch_area_forced

real(r8), parameter, public :: min_nppatch = min_npm2*min_patch_area ! minimum number of cohorts per patch (min_npm2*min_patch_area)
real(r8), parameter, public :: min_n_safemath = 1.0E-12_r8 ! in some cases, we want to immediately remove super small
real(r8), parameter, public :: min_n_safemath = 1.0E-15_r8 ! in some cases, we want to immediately remove super small
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this change? assuming related to the prior one?

main/FatesHistoryInterfaceMod.F90 Outdated Show resolved Hide resolved
parameter_files/fates_params_default.cdl Outdated Show resolved Hide resolved
parameter_files/fates_params_default.cdl Outdated Show resolved Hide resolved
main/EDTypesMod.F90 Outdated Show resolved Hide resolved
@@ -340,6 +342,9 @@ module EDTypesMod
! (below ground)
real(r8) :: froot_mr ! Live fine root maintenance respiration: kgC/indiv/s

!DAMAGE
real(r8) :: branch_frac ! Fraction of aboveground biomass in branches
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand why branch_frac needs to be tracked as a cohort-level state variable, rather than a thing that is calculated as a function of PFT parameters and crown damage class? Does it? Also, isn't this the fraction of aboveground woody biomass that is in branches, not the fraction of total aboveground biomass in branches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've simplified the code so that branch_frac is no longer a cohort variable. Also updated comments where it is declared.

JessicaNeedham and others added 21 commits December 9, 2021 09:44
[ ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [ No]

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
…ases

when damage module is on. ]

[don't change maxCohortsPerPatch in EDTypesMod but instead in FatesInterfaceMod ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [No]

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
[Increased it to 3 to work with Ryan's bci param file but
reverting it to 2 for PR ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [No]

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
[Can't remember why I changed these. Possibly to do with termination mortality?  ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ Remove the crowndamage x crowndamage dimension,
remove unecessary history variables and put all
damage history variables in an if statement ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
…ge_module' into JessicaNeedham-crowndamage_module
[ ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ crowndamage to crown damage ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
frequency of damage events. ]

[ ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
damage history outputs ]

[ Remove damage only history dimension. Fix bug with
crownarea by damage and mortality. ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ update damage history variable units to be in line
with recent history output refactor ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ Branch frac does not need to be associated with cohorts.
This simplifies many of the calls to allometry functions. ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
Conflicts:
	biogeochem/EDCanopyStructureMod.F90
	biogeochem/FatesAllometryMod.F90
	main/EDParamsMod.F90
	main/EDTypesMod.F90
[Add cohort  crowndamage status to calls to tree_sai ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ Needed for damage transition matrix ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
[ because damage history variables are defined in an if statment
they need to be references by this%hvars and not hio_ ]

Fixes: [NGT-ED Github issue #]

User interface changes?: [Yes (describe what changes), No]

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
Conflicts:
	main/FatesIODimensionsMod.F90
	main/FatesInterfaceMod.F90
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.

Initial review due to regression test failure in debug mode. One other minor suggestion as well.

biogeophys/FatesPlantRespPhotosynthMod.F90 Outdated Show resolved Hide resolved
main/FatesParameterDerivedMod.F90 Outdated Show resolved Hide resolved
glemieux and others added 4 commits October 24, 2022 14:53
Refactor damage check to enable the use of `sap_c_` variables regardless of run mode
Fixes exact restart issues with the vegetation temperature for run modes with bareground patches
@glemieux
Copy link
Contributor

glemieux commented Oct 25, 2022

Regression testing on Cheyenne is complete. All expected tests pass b4b. Note that the tree damage test mod which wasn't generating baselines prior to this PR is now running and generating baselines successfully. Also, the addition of MORTALITY_CROWNAREA_CANOPY and MORTALITY_CROWNAREA_UNDERSTORY is resulting in expected FIELDLIST DIFFs for those test mods that includes all active history output.

One minor clarification: for some reason the P144x2 mimics fates test mod didn't have a baseline when I ran the initial comparison, which resulted in an NLCOMP DIFF and BFAIL. I regenerated that baseline and re-ran the comparison which is comping back b4b, but the NLCOMP DIFF is still showing up with the automated check. Looking at the TestStatus.log for that test, you can see these diffs are from that original failed comparison against the missing baseline and thus can be ignored.

Folder location: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr788-crowndamage-2

@glemieux glemieux merged commit 3374ba3 into NGEET:master Oct 31, 2022
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 this pull request may close these issues.

4 participants