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

Fire mortality diagnostics update #431

Merged
merged 21 commits into from
Jan 12, 2019
Merged

Conversation

jkshuman
Copy link
Contributor

@jkshuman jkshuman commented Oct 18, 2018

Description:

Included in this PR is a fix to issue #408 correcting a typo to set the diffuse surface albedo as identified by J. Holm within EDSurfaceAlbedoMod.
These changes emerged from issue #412 . @ckoven updated the fire mortality diagnostics calculations. Specifically, the rates of fire mortality, rate of cambial mortality (from fire damage) and rate of crown mortality (due to fire damage) are tracked for diagnostics purposes. Site-level fire mortality is further stratified by canopy level. J.K. Shuman reviewed the changes by C. Koven, updated a typo to correct the association between cambial mortality and crown mortality, and removed the hard coding of canopy layers for the site-level stratification.
This PR was updated to include the changes described in issue #436 by removing the calculated value alpha_FMC from the parameter file and adding the parameter drying ratio to the parameter file. alpha_FMC is calculated using drying_ratio and SAV (both from the parameter file). This update removes the possibility of having SAV and alpha_FMC values that are not related due to user error (forgetting to update both) by placing the calculation within the fuel moisture subroutine of SPITFIRE.

Collaborators:

JKShuman, CKoven, RKnox, RFisher, JHolm

Expectation of Answer Changes:

Answer changes are expected, specifically due to update of diffuse albedo.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
  • [x ] I have read the CONTRIBUTING document.
  • [x ] FATES PASS/FAIL regression tests were run
  • If answers were expected to change, evaluation was performed and provided

Test Results:

This version compiles and runs on Cheyenne, but fails with a carbon balance error for the file set that I have in year 1 month 4. It failed the basegen tests on Cheyenne. Failure of basegen tests was discussed with @rgknox

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

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

FATES baseline hash-tag:

jkshuman and others added 5 commits September 25, 2018 15:05
…so added cambial and crown fire mort diagnostci variables
Update typos within patch dynamics to link cambial and crown scorch mortality for diagnostics.
Remove hard coding of canopy layer

Fixes: 412

User interface changes?: No

Code review: JKShuman, CKoven, RKnox, RFisher

Test suite: Hobart
Test baseline:
Test namelist changes:
Test answer changes: expect changes
Test summary: Run on Hobart and then test on Cheyenne.
Correct typo to set the diffuse surface albedo correctly as identified by JHolm

Fixes:408

User interface changes?: No

Code review: JKShuman, JHolm, RKnox, RFisher

Test suite: Hobart and Cheyenne
Test baseline:
Test namelist changes: none
Test answer changes:expect changes
Test summary: will run on Hobart and follow with baseline tests on Cheyenne
@rgknox rgknox requested review from ckoven and rgknox October 23, 2018 21:09
@rgknox rgknox self-assigned this Oct 23, 2018
Copy link
Contributor

@rgknox rgknox left a comment

Choose a reason for hiding this comment

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

The submitted changes are a welcomed improvement. Thanks @jkshuman and @ckoven . I can help test to see if the fire diagnostics need to be in the restart. However, see my notes in-line about the canopy vs understory indexing, and summing.

@@ -107,7 +107,6 @@ module FatesRestartInterfaceMod
integer, private :: ir_bmort_co
integer, private :: ir_hmort_co
integer, private :: ir_cmort_co
integer, private :: ir_fmort_co
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 wondering if we need to add the new fire mortality arrays to the restarts... I suppose we will see if they were necessary after we run the tests.

Copy link
Contributor Author

@jkshuman jkshuman Jan 9, 2019

Choose a reason for hiding this comment

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

@rgknox Can you provide more details on this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

For instance at this line:
https://github.com/NGEET/fates/pull/431/files#diff-682b3e29ce096fa74f8fa95ff48735c4R2164

We are updating a history diagnostic there. We have to make sure that these arrays are correctly initialized after restarting the model. Will update code accordingly.

main/FatesHistoryInterfaceMod.F90 Outdated Show resolved Hide resolved
rgknox and others added 4 commits October 24, 2018 16:45
conflict resolutions performed by @rgknox to bring this branch up to speed with parteh-master. This will make testing and eventual merge into master much faster.
@rgknox
Copy link
Contributor

rgknox commented Jan 8, 2019

Pending @jkshuman 's merging of some changes, tests on cheyenne pass, b4b with master as well:
/glade/scratch/rgknox/clmed-tests/fates.cheyenne.intel.fire-mort-diags-v2-C5dc7f0a-F2b5330b

jkshuman and others added 3 commits January 8, 2019 10:27
conflict resolutions and fixes to canopy layer diagnostics
With this update, a change in SAV will result in an updated
alpha_FMC based on new calcualtion from SF_drying_ratio to
calculate weighted avg of relative fuel moisture content
fire/SFMainMod.F90 Outdated Show resolved Hide resolved
Jacqueline Shuman and others added 3 commits January 8, 2019 13:34
@@ -607,17 +608,25 @@ module EDTypesMod

! TERMINATION, RECRUITMENT, DEMOTION, and DISTURBANCE

Copy link
Contributor

Choose a reason for hiding this comment

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

@ckoven , can you sign-off that changing terminated_nindivs and termination_carbonflux from 2 layers to nclmax is ok with you? You will see changes in the code that also reflect this change in usage, and are not separated into upper and not-upper. I changed this because it was confusing to me on first if these arrays were allocated by canopy layer or not, and for clarity and consistency sake, changed it to be by canopy layer since that is our typical convention. We can revert it if you feel these arrays are better allocated as the binary form, but we need to give them an integer parameter that describes this dimension if so.

Copy link
Contributor

Choose a reason for hiding this comment

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

From email exchange with @ckoven:

"great. so long as we do that everywhere, it works for me. "

@jkshuman
Copy link
Contributor Author

jkshuman commented Jan 9, 2019

@rgknox I am done merging changes and updates. This PR successfully runs with and without fire across South America with a tropical tree and grass. Both runs completed through 42 years successfully with reasonable biomass accumulation for trees and grasses. Let me know if/how you want to address your earlier comment regarding fire mortality arrays in the restarts.

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.

3 participants