two-stream - b4b Norman and no history changes #1141
Conversation
…iver code and module names.
…egative absorption
…dated netcdf python tools to use updated scipy library configuration
…opy and radiation compositions that may had failed or are problematic
…and albedo correction
rgknox
force-pushed
the
two-stream-clean-b4b-nohist
branch
from
20a1b22 to
9c6dcae
Compare
rgknox
force-pushed
the
two-stream-clean-b4b-nohist
branch
from
9c6dcae to
312593f
Compare
| real(r8) :: remainder ! old-method: remainder of biomass in last bin | ||
| integer, parameter :: layer_height_const_depth = 1 ! constant physical depth assumption | ||
| integer, parameter :: layer_height_const_lad = 2 ! constant leaf area depth assumption | ||
| integer, parameter :: layer_height_method = layer_height_const_depth |
There was a problem hiding this comment.
What's the background here on the layer_height_method? Will one method be choosen later or be converted to a formal parameter (i.e. moved into the parameter file)? A think a few code comments here explaining the rationale for the hard coded value would be helpful.
There was a problem hiding this comment.
The constant depth assumption is the default, and is equivalent to the way we currently do things in the canopy summarization code. I added the alternative so that we have the option to explore it in the future. I have not done a thorough investigation of it yet though.
biogeochem/FatesPatchMod.F90
Outdated
| @@ -112,39 +114,36 @@ module FatesPatchMod | |||
| real(r8) :: c_lblayer ! mean boundary layer conductance of all leaves in the patch [umol/m2/s] | |||
|
|
|||
| !TODO - can we delete these? | |||
There was a problem hiding this comment.
We can delete this comment now, correct, since your did consolidate the nrmlzd variables?
| real(r8), parameter :: lmrc = 1.15912391_r8 ! scaling factor for high | ||
| ! temperature inhibition (25 C = 1.0) | ||
|
|
||
| !veg_tempk = 27._r8+271._r8 |
There was a problem hiding this comment.
Is this development code that is no longer valid? If so, we should reimplement the intent option on veg_tempk dummy argument definition up above (line 2232).
main/FatesIOVariableKindMod.F90
Outdated
| integer, parameter, public :: upfreq_dyn = 1 ! dynamics variables | ||
| integer, parameter, public :: upfreq_hifr = 2 ! high frequency variables | ||
| !integer, parameter, public :: upfreq_dyn_multi = 3 ! dynamics multi-dimension | ||
| integer, parameter, public :: upfreq_hydr = 4 ! Hydro variables | ||
| integer, parameter, public :: upfreq_nutr = 5 | ||
| integer, parameter, public :: upfreq_hifr_multi = 6 ! high frequency multi-dim |
There was a problem hiding this comment.
This appears to be pulled over from #1119. Since they're not used it doesn't seem to be a big deal, but it might be worth removing for that forthcoming PR,
radiation/FatesRadiationMemMod.F90
Outdated
| ! For now, this module also holds relevant data for Norman radiation | ||
| ! --------------------------------------------------------------------------- | ||
|
|
||
| integer, parameter :: r8 = selected_real_kind(12) |
There was a problem hiding this comment.
This is replicated to support unit testing?
| @@ -267,7 +266,7 @@ module FatesInterfaceTypesMod | |||
| integer , public, allocatable :: fates_hdim_levfuel(:) ! fire fuel size class (fsc) dimension | |||
| integer , public, allocatable :: fates_hdim_levcwdsc(:) ! cwd class dimension | |||
| integer , public, allocatable :: fates_hdim_levcan(:) ! canopy-layer dimension | |||
| integer , public, allocatable :: fates_hdim_levleaf(:) ! leaf-layer dimension | |||
| real(r8), public, allocatable :: fates_hdim_levleaf(:) ! leaf-layer dimension, integrated VAI [m2/m2] | |||
There was a problem hiding this comment.
Was this updated to avoid the silent type conversion from real to integer since it's referencing dlower_vai?
| @@ -0,0 +1,533 @@ | |||
| Module FatesTwoStreamInterfaceMod | |||
There was a problem hiding this comment.
Per our discussion, reword Interface to Utils (or other word) to avoid confusing this as being related to other API interface modules.
radiation/TwoStreamMLPEMod.F90
Outdated
|
|
||
| ! ================================================================================================ | ||
|
|
||
| subroutine ParamPrep() |
There was a problem hiding this comment.
Minor nitpick to make the procedure more distinct from other type of parameters.
| subroutine ParamPrep() | |
| subroutine RadParamPrep() |
Description:
This set of changes is a modification of #1036. Updates have been made to ensure that the same results are achieved with Norman radiation. Also, changes to the history file will be applied in a later PR, so this has been reverted as well.
Collaborators:
@glemieux and same folks from #1036
Expectation of Answer Changes:
No answer changes, but possibility of very very small diffs. Tests so far on Derecho have achieved b4b
Checklist
If this is your first time contributing, please read the CONTRIBUTING document.
All checklist items must be checked to enable merging this pull request:
Contributor
Note: documentation has been written. It needs to be added to the documentation.
Integrator
Test Results:
More tests forthcoming, but preliminary fates test suite on derecho is b4b with ctsm5.1.dev159-sci.1.69.0_api.31.0.0: /glade/derecho/scratch/rgknox/tests_1219-132458de/
CTSM (or) E3SM (specify which) test hash-tag:
CTSM (or) E3SM (specify which) baseline hash-tag:
FATES baseline hash-tag:
Test Output: