Improve the plant hydro codes to accomodate dynamic forest #440
Conversation
…over effects on hydraulics
…ssing cohort-level variables
… check for the hydraulics switch to be true, so will prob cause crash when hydraulics turned off
… ease of matching additions made to CopyCohortHydraulics
…pft-specifc parameters; added back in original code for innermost rhiz shell kmax for testing purposes
… Hijacked the scagpft dimension (temporary measure)
… but left commented out due to circular dependency issues with EDPftvarcon
…the amount of water in new recruits
…nt of water in new recruits. Subtract this from bc_out%plant_stored_h2o_si in UpdateH2OVeg call only.
…cohorts and fixes a bug elsewhere in a call to this subroutine
…com:xuchongang/fates-1 into xuchongang/fates_hydro_improve_ready_to_merge
…_hydro_improve_ready_to_merge
for the dump_cohort for hydro to avoid cycirlar module references
…xuchongang/fates into xuchongang/fates_hydro_improve_ready_to_merge
|
Thanks for providing the hydro default parameter file @xuchongang. Can we now consider this PR "ready" for testing and peer review? |
|
@rgknox , I think it is ready for testing and peer review. I have also added the hydraulic failure mortality based on hydro. |
| @@ -86,6 +86,7 @@ module EDPftvarcon | |||
| real(r8), allocatable :: mort_scalar_cstarvation(:) | |||
| real(r8), allocatable :: mort_scalar_hydrfailure(:) | |||
| real(r8), allocatable :: hf_sm_threshold(:) | |||
| real(r8), allocatable :: hf_flc_threshold(:) | |||
There was a problem hiding this comment.
We don't have the best precedent on this, but it would be awesome if we could have a comment after this definition that describes the parameter and its units. See, #437
| @@ -77,12 +84,28 @@ subroutine mortality_rates( cohort_in,bc_in,cmort,hmort,bmort,frmort ) | |||
|
|
|||
| ! Proxy for hydraulic failure induced mortality. | |||
| hf_sm_threshold = EDPftvarcon_inst%hf_sm_threshold(cohort_in%pft) | |||
|
|
|||
| if(cohort_in%patchptr%btran_ft(cohort_in%pft) <= hf_sm_threshold)then | |||
| hf_flc_threshold = EDPftvarcon_inst%hf_flc_threshold(cohort_in%pft) | |||
There was a problem hiding this comment.
At first I thought maybe we should consider having just one parameter here, generically defined as "hydraulic failure threshold". But it does appear that the units of hf_sm_threshold and hf_flc_threshold are completely different. One is soil moisture and the other is conductivity. I think I like them separated into two different values. Maybe we should add a note in the parameter file meta-data and the parameter definitions "in-code" as to where they are relevant though.
main/FatesHistoryInterfaceMod.F90
Outdated
| @@ -1511,7 +1518,12 @@ subroutine update_history_dyn(this,nc,nsites,sites) | |||
| hio_ddbh_canopy_si_scag => this%hvars(ih_ddbh_canopy_si_scag)%r82d, & | |||
| hio_ddbh_understory_si_scag => this%hvars(ih_ddbh_understory_si_scag)%r82d, & | |||
| hio_mortality_canopy_si_scag => this%hvars(ih_mortality_canopy_si_scag)%r82d, & | |||
| hio_mortality_understory_si_scag => this%hvars(ih_mortality_understory_si_scag)%r82d) | |||
| hio_mortality_understory_si_scag => this%hvars(ih_mortality_understory_si_scag)%r82d, & | |||
| hio_h2oveg_dead_si => this%hvars(ih_h2oveg_dead_si)%r81d, & | |||
There was a problem hiding this comment.
@xuchongang, I'm running tests on cheyenne and running into problems at this line.
The problem is that these diagnostics are only allocated if hydraulics is turned on, yet this routine wants to associate a pointer to them even when hydraulics is off, and it is returning errors.
However, I'm wondering, do we even want these diagnostics calculated in this subroutine? This is only called once per day, but my impression was that these water contents are updated sub-daily right? Shouldn't we move these calculations to update_history_hydraulics anyway?
There was a problem hiding this comment.
After looking at this again, it looks like these should be updated sub-daily:
ih_h2oveg_si
ih_h2oveg_hydro_err_si
And these should be updated daily:
ih_h2oveg_dead_si
ih_h2oveg_recruit_si
ih_h2oveg_growturn_err_si
ih_h2oveg_pheno_err_si
Seem right to you @xuchongang @bchristo ?
… only accessed if turned on and allocated
…y_to_merge merge resolution (trivial)
…y_to_merge conflict resolutions
|
Tests run on Cheyenne: A new exact restart test was added to CTSM for dynamic hydro. While the "compare" phase does not pass, I think nailing down restart capabilities is more of a medium term goal. My objective in getting the restart machinery into the current PR was more about staging, and not completing the task. Will create an issue. |
| write(fates_log(),*)'Error: psi_note become positive,& | ||
| psi_node=',psi_node |
There was a problem hiding this comment.
Please note that IBM compiler issues a build error on a string split into multiple lines without a continuation character on the next line: i.e. need to to update this to either
write(fates_log(),*)'Error: psi_note become positive,&
& psi_node=',psi_node
or just
write(fates_log(),*)'Error: psi_note become positive, psi_node=',psi_node
There was a problem hiding this comment.
@amametjanov this is something we can do and will try to enforce, thanks for bringing this up
Improve the plant hydro codes to accomodate dynamic forest
Description:
This pull request updates the hydro codes to accomplish the following key copmonents:
Collaborators:
@bchristo , @ckoven , @rosiealice , @rgknox
Expectation of Answer Changes:
This should make the HYDRO code works for the dynamics mode.
Checklist:
Test Results:
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: