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

Rosie colddec #6

Merged
merged 8 commits into from
Oct 1, 2019
Merged

Rosie colddec #6

merged 8 commits into from
Oct 1, 2019

Conversation

rosiealice
Copy link

Description:

Collaborators:

Expectation of Answer Changes:

  • [ x] 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:

@@ -907,7 +931,7 @@ subroutine phenology_leafonoff(currentSite)
real(r8) :: store_c_transfer_frac ! Fraction of storage carbon used to flush leaves
integer :: ipft
real(r8), parameter :: leaf_drop_fraction = 1.0_r8

real(r8), parameter :: carbon_store_buffer = 0.10_r8
Copy link
Owner

Choose a reason for hiding this comment

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

@rosiealice , would it be most desirable to add this to the parameter file? We could flag this to be added whenever we have a critical mass of changes to implement.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe, but

  1. it's really there to stop us triggering the termination mortality logic, and so is perhaps better thought of as some sort of internal maths?
    however:

  2. Taking the plant into a very low storage state might in preinciple trigger carbon starvation mortality, even though this is really how deciduous trees are 'supposed' to work. I couldn't really follow the logic of how the target C was calculated in PARTEH well enough to know if there was dispensation for deciduous trees, but irrespective, we don't want to go around killing trees that are really just doing their deciduous things. Then that raises the question of how we -do- get carbon starvation to happen in deciduous trees, and whether those trees should all have a target storage >>1...

  3. Having this as a parameter would allow interrogation of the 'save carbon for a drought' vs. 'put it all in the display pools in the spring' strategies.

! In the north, don't accumulate when we are past the leaf fall date.
! Accumulation starts on day 1 of year in NH.
! The 180 is to prevent going into an 'always off' state after initialization
if( model_day_int .gt. currentSite%cleafoffdate.and.hlm_day_of_year.gt.180)then !
Copy link
Owner

Choose a reason for hiding this comment

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

@rosiealice , I think it would be nice to give 180 an explicit constant name as well.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, although the 'start of counting date' might be more tractable and we could key the 180 off that?

Copy link
Owner

Choose a reason for hiding this comment

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

sounds good

@rgknox rgknox merged commit 5809fb5 into rgknox:fix-leafstatus Oct 1, 2019
rgknox pushed a commit that referenced this pull request Feb 4, 2020
merge up to tag 1.31.1_8.1.0 and add file version of cg_strikes
glemieux pushed a commit that referenced this pull request Jul 10, 2023
rgknox pushed a commit that referenced this pull request Oct 25, 2023
Fix parameter file read bug as well as old parameter file overwrite code
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.

2 participants