Skip to content

Conversation

@xuchongang
Copy link
Contributor

@xuchongang xuchongang commented Jul 19, 2019

This pull request update the phenology for grass. Namely, to drop stems during cold or drought conditions.

Description:

We made the following key modifications:

  1. add phen_stem_drop_fraction into the parameter file and EDPhysiologyMod so that the grass can drop their stem during winter or drought;
  2. make the leaf phenology for grass size-dependent. This modification is made so that it can accommodate the situation of small salt marsh can overwinter due to the protection of tall dead stems.

Collaborators:

Lu Zhai, [email protected]

Expectation of Answer Changes:

For grass, it will change the phenology if phen_stem_drop_fraction set to values > 0. Otherwise, it should generate the same output as before.

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.
  • FATES PASS/FAIL regression tests were run
  • If answers were expected to change, evaluation was performed and provided

Test Results:

Not yet

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

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

FATES baseline hash-tag:

Test Output:

xuchongang and others added 30 commits March 7, 2019 14:52
… DBH threshold value for cold induced leaf/stem drop
… of the stem drop to the fates parameter file
@glemieux
Copy link
Contributor

My local branch of this PR is failing on the exact restart test for the mpi-serial library. It looks like sapwmemory is being set to NaN instead of zero or the previous value. Currently investigating.

@glemieux
Copy link
Contributor

@LuZhai @xuchongang I noticed in EDInitMod that the init_cohorts subroutine does not include code for sapwmemory or structmemory for deciduous stress:

fates/main/EDInitMod.F90

Lines 506 to 522 in bfe83fd

if( EDPftvarcon_inst%season_decid(pft) == itrue .and. &
any(site_in%cstatus == [phen_cstat_nevercold,phen_cstat_iscold])) then
temp_cohort%laimemory = c_leaf
temp_cohort%sapwmemory = c_sapw * stem_drop_fraction
temp_cohort%structmemory = c_struct * stem_drop_fraction
c_leaf = 0._r8
c_sapw = (1.0_r8-stem_drop_fraction) * c_sapw
c_struct = (1.0_r8-stem_drop_fraction) * c_struct
cstatus = leaves_off
endif
if ( EDPftvarcon_inst%stress_decid(pft) == itrue .and. &
any(site_in%dstatus == [phen_dstat_timeoff,phen_dstat_moistoff])) then
temp_cohort%laimemory = c_leaf
c_leaf = 0._r8
cstatus = leaves_off
endif

whereas in FatesInventoryInitMod, there is matching code for both stress and seasonality in set_inventory_edcohort_type1:

if( EDPftvarcon_inst%season_decid(temp_cohort%pft) == itrue .and. &
any(csite%cstatus == [phen_cstat_nevercold,phen_cstat_iscold])) then
temp_cohort%laimemory = b_leaf
temp_cohort%sapwmemory = b_sapwood * stem_drop_fraction
temp_cohort%structmemory = b_dead * stem_drop_fraction
b_leaf = 0._r8
b_sapwood = (1._r8 - stem_drop_fraction) * b_sapwood
b_dead = (1._r8 - stem_drop_fraction) * b_dead
cstatus = leaves_off
endif
if ( EDPftvarcon_inst%stress_decid(temp_cohort%pft) == itrue .and. &
any(csite%dstatus == [phen_dstat_timeoff,phen_dstat_moistoff])) then
temp_cohort%laimemory = b_leaf
temp_cohort%sapwmemory = b_sapwood * stem_drop_fraction
temp_cohort%structmemory = b_dead * stem_drop_fraction
b_leaf = 0._r8
b_sapwood = (1._r8 - stem_drop_fraction) * b_sapwood
b_dead = (1._r8 - stem_drop_fraction) * b_dead
cstatus = leaves_off
endif

Should EDInitMod have the initialization code for sapwmemory and structmemory for deciduous stress?

@xuchongang
Copy link
Contributor Author

xuchongang commented Jan 24, 2020

@glemieux , good catch and we need to let EDInitMod have the initialization code for sapwmemory and structmemory for deciduous stress.

@glemieux
Copy link
Contributor

@glemieux , good catch and it we need to have EDInitMod have the initialization code for sapwmemory and structmemory for deciduous stress.

Thanks; I'll add it on my local branch. I fixed the restart error as well just now. New PR coming to your branch sometime later today.

@glemieux glemieux added science: bug Bugs that are specific to the implementation of a scientific model and removed science: bug Bugs that are specific to the implementation of a scientific model labels Jan 24, 2020
@glemieux
Copy link
Contributor

That was interesting. I somehow added labels using keyboard shortcuts. That's what I get for using two screens and not paying attention to both

@xuchongang
Copy link
Contributor Author

@glemieux, thanks and I did not see the pull request on my branch yet.

@glemieux
Copy link
Contributor

@xuchongang apologies for the delay. I spoke too soon; I wanted to wait for the test to complete, which was held up until the weekend in the Cheyenne queue. The test passed and I've created the PR to your branch.

@xuchongang
Copy link
Contributor Author

@glemieux, thank you and I have made the merge of your pull request.

@glemieux
Copy link
Contributor

glemieux commented Jan 29, 2020

Testing update:

All standard testing passed B4B with the default parameter file (i.e. phen_stem_drop_frac = 0) except for the SMS_Lm6.f45_f45 test which failed on a DIFF to the baseline. @rgknox and I took a look at this yesterday and we're still struggling to determine the reason for the difference. Test output can be found here:

/glade/u/home/glemieux/scratch/clmed-tests/pr554-xuchongang-grass-phen.fates.cheyenne.intel.C5ad8a8d2-Fb9a54836

I also ran a parallel 5 year test at Brazil with and without the stem drop fraction greater than zero (in this case I set all PFTs to a randomly selected value of 0.25). Output comparison plots run through the ACRE suite can be found here: https://drive.google.com/file/d/1Bj5gBPf7a13bvrG7X01ho2V6jCaiimBL/view?usp=sharing

@glemieux
Copy link
Contributor

At @ckoven I plotted out the SMS_Lm6.f45_f45 differences between the baseline and the PR branch.

GPP:
GPP

TLAI:
TLAI

From this and the cprnc output it looks like the problem is just with the one grid cell, for this time point at least. @rgknox suggestion I've got a longer term f45 grid running to see if the errors grow. Barring an expansion of the diff to other grids and/or the increase in the difference, do you think this is reasonable to merge, @xuchongang ?

@xuchongang
Copy link
Contributor Author

@glemieux , for me, it is fine to merge.....

@glemieux
Copy link
Contributor

glemieux commented Feb 3, 2020

@rgknox had another thought on what might be causing the non-B4B results even when the stem drop parameter is zero. The flushing logic looks like it could possibly flush only a portion of the store_c_transfer_frac since it's only checking for non-woody plants: https://github.com/NGEET/fates/pull/554/files#diff-b2580903c042bbb143bb267a7b882e3fR990-R1009

At this suggestion, I added a check during the flush to use all of the store_c_transfer_frac for leaves if the stem drop fraction is zero:
Drought leaf on section: https://github.com/glemieux/fates/blob/ca148d0ff5d4dd0470d17d9e51da2e2e90a005cd/biogeochem/EDPhysiologyMod.F90#L990-L1020
Cold leaf on section: https://github.com/glemieux/fates/blob/ca148d0ff5d4dd0470d17d9e51da2e2e90a005cd/biogeochem/EDPhysiologyMod.F90#L1103-L1135

This indeed resulted in B4B results. The question is, should we be using totalmemory for leaves when stem drop is zero like I have done here? Or is this changing the intent of the hypothesis?

@xuchongang
Copy link
Contributor Author

@glemieux , I think "using totalmemory for leaves when stem drop is zero" sounds reasonable to me.

Add stem drop fraction check for flushing
@glemieux
Copy link
Contributor

glemieux commented Feb 4, 2020

Final testing results. All expected PASS:

/glade/u/home/glemieux/scratch/clmed-tests/pr554-phen-flush-b4b-fix-Cf8f641ab-Fca148d0f.fates.cheyenne.intel

@glemieux glemieux merged commit 90e4639 into NGEET:master Feb 4, 2020
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