parteh v1 #421

rgknox · 2018-10-03T05:02:00Z

Description:

This changeset contains the first implementation of the Plant Allocation and Reactive Transport Extensible Hypotheses. The parteh system is designed to enable flexibility in which organs and species are enabled in a plant allocation hypothesis, and also how many discrete spatial positions for those organs are enabled.

Some notes:

Only the carbon-only hypothesis, pre-existing, is implemented
Generic/object looping for populating and reading restarts has been added, but nothing special for history output
A functional unit testing framework has also been added, see the new folder functional_unit_testing/parteh
New and changed parameters
Re-translocation of C&N&P has been added
Helper functions for event based leaf-drop, leaf-flush, burning and seed drop have been added

Synced with: ESCOMP/CTSM#537

Documentation PR: NGEET/fates-docs#13

Collaborators:

@ckoven, @bishtgautam , @rosiealice , @xuchongang , @jkshuman , @tompowell9 , @bchristo , @jenniferholm

Expectation of Answer Changes:

Answers are expected to change, but only slightly. So much code has changed, but it is intended to generate the same answers within reasonable precision.

Checklist:

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:

30 Year regression test against master at BCI (no inventory): https://drive.google.com/file/d/1HwoBhMw1QBR_2qcCyzTmagouismT_C9b/view?usp=sharing

FATES test suite TBD

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

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

FATES baseline hash-tag:

Test Output:

…on pass. Not hooked into FATES though.

…e (redundant).

…nit testing framework.

…e the CNP hypothesis (external).

…y and needs cleanup.

…arbon inbalance.

…usion weighting

…art. Untested

rgknox · 2018-10-03T05:06:18Z

FYI: I plan to go through and do a few more passes, cleaning and commenting code. But I wanted to get this PR established so others can familiarize with changes at earliest convenience.

As you can see in the the acre tests, this seems to be generating fairly similar results compared to master, and seems to be stable. More testing is on its way, including a multi-decade f45 smoke test.

…on parameter, and checks.

rgknox · 2018-10-09T19:07:06Z

Conducted another test using the setup described in #424 (gridded run over South America, 80 year simulation, tropical evergreen and c3 stress deciduous grass).

TreeGrass-parteh-v2_plots.pdf

serbinsh · 2018-10-18T18:45:38Z

biogeochem/EDCanopyStructureMod.F90

@@ -1017,7 +1080,7 @@ subroutine canopy_summarization( nsites, sites, bc_in )
             call carea_allom(currentCohort%dbh,currentCohort%n,sites(s)%spread,&
                  currentCohort%pft,currentCohort%c_area)

-             currentCohort%treelai = tree_lai(currentCohort%bl,             &
+             currentCohort%treelai = tree_lai(leaf_c,             &
                  currentCohort%pft, currentCohort%c_area, currentCohort%n, &


Apologies for this additional naive question but I dont see SLA in this subroutine/function to calculate LAI for each cohort, or is that the c_area?

@kovenock updated the LAI calculations to use SLA at the top of the plant, and optionally a higher SLA that increases as one decreases in elevation in the canopy. But at its heart, the tree_lai() routine should be making use of these parameters. It is a parameter, not a dynamic quantity, so it does not need to be passed in as an argument.

Got it, thanks!

@rgknox as far as my comment about bleaf vs leaf_c, here is one of the examples using leaf_c. Just wanted to make sure whether you had planned for everything to be leaf_c or if in the other case bleaf is appropriate?

My take is that we should drop the b_ convention altogether, except we want to describe things in terms of total biomass (including not just carbon, but lignans, water, nutrients, etc). We do have a subroutine called bleaf(), and I want to change that too. That subroutine is actually calculating the plants target leaf carbon for a fully flushed, on-allometry, plant of a given size and trimming. So bleaf(), is not a very concise subroutine name.

serbinsh · 2018-10-18T18:48:10Z

biogeochem/EDCohortDynamicsMod.F90

@@ -150,7 +204,7 @@ subroutine create_cohort(currentSite, patchptr, pft, nn, hite, dbh, bleaf, bfine
    ! Assign canopy extent and depth
    call carea_allom(new_cohort%dbh,new_cohort%n,spread,new_cohort%pft,new_cohort%c_area)

-    new_cohort%treelai = tree_lai(new_cohort%bl, new_cohort%pft, new_cohort%c_area,    &
+    new_cohort%treelai = tree_lai(bleaf, new_cohort%pft, new_cohort%c_area,    &


Just curious why the calculation of LAI here is slightly different than how it is done in biogeochem/EDCanopyStructureMod.F90 where here we have bleaf (leaf biomass) and above leaf_c which is also leaf biomass? Just trying to understand the steps....

@serbinsh , the stuff that mostly happens in EDCanopyStructureMod, is the tallying of LAI from each cohort, and assembling those LAI's into canopy, leaf and PFT bins within each patch. But that part of the code shouldn't be calculating the individual plant's LAI any differently. Could you point to the line in question so that I can double check?

serbinsh · 2018-10-18T18:52:31Z

biogeochem/EDCohortDynamicsMod.F90

+          call SetState(currentCohort%prt,sapw_organ,carbon12_species,0.0_r8)
+          call SetState(currentCohort%prt,struct_organ,carbon12_species,0.0_r8)
+          call SetState(currentCohort%prt,repro_organ,carbon12_species,0.0_r8)
+          call SetState(currentCohort%prt,store_organ,carbon12_species,0.0_r8)


So here we we set these pools we are using carbon12 (specific isotopic version) and above we are just using "all_carbon_species" Here we would allow for isotopic differentiation?

Right, good question. When we are using SetState(), we are initializing a specific variable, we can't set a set of them. But in most other cases, when we are querying, we just want to know the total mass, so we use "all_carbon_species" to signal that we want all carbon isotopes.

If you have a scenario, where on the FATES side of things you want to ask PARTEH for the leaf C13 for instance, you could call prt%GetState(leaf_organ,carbon13_species). ... Assuming that you have that isotope enabled in the specify PARTEH hypothesis you have turned on.

serbinsh · 2018-10-18T18:54:17Z

biogeochem/EDMortalityFunctionsMod.F90

@@ -57,7 +59,8 @@ subroutine mortality_rates( cohort_in,bc_in,cmort,hmort,bmort,frmort )
    real(r8),intent(out) :: frmort ! freezing stress mortality

    real(r8) :: frac  ! relativised stored carbohydrate
-    real(r8) :: b_leaf ! target leaf biomass kgC
+    real(r8) :: leaf_c_target      ! target leaf biomass kgC


so again to clear up some possible confusion, we seem to still be interchanging b_leaf with leaf_c in some places, is that correct? Or should we replace all previous b_leaf with leaf_c to represent leaf biomass?

these naming conventions should definitely be cleared up. I'm pretty sure that anywhere in the code you see "b_leaf", it actually is referring to the target leaf biomass, ie the value of the pool when it is on allometry and fully flushed.

serbinsh · 2018-10-18T18:55:09Z

biogeochem/EDMortalityFunctionsMod.F90

-       call bleaf(cohort_in%dbh,cohort_in%pft,cohort_in%canopy_trim,b_leaf)
-       call storage_fraction_of_target(b_leaf, cohort_in%bstore, frac)
+
+       call bleaf(cohort_in%dbh,cohort_in%pft,cohort_in%canopy_trim,leaf_c_target)


Or maybe I am accidentally conflating things since I also see this sub is called bleaf.

that subroutine should be renamed to convey that it is returning the allometric target leaf carbon

aha! that makes sense then. I agree renaming would reduce confusion

serbinsh · 2018-10-18T18:56:57Z

biogeochem/EDPatchDynamicsMod.F90

          if(EDPftvarcon_inst%woody(currentCohort%pft) == 1)then
-             burned_leaves = currentCohort%bl * currentCohort%fraction_crown_burned
+             burned_leaves = leaf_c * currentCohort%fraction_crown_burned


So here leaf_c is a mass and this X area fraction gives remaining mass of burned leaves?

It's not an area fraction, it's more like a fraction of the crown burned, calculated from flame height.

so a 1D fraction so say, 5kg of leaf biomass * 40% burned crown = 2 kg of living biomass / 3 kg of burned? where burned_frac = f(flame height)?

Yes, that sort of thing.

serbinsh · 2018-10-18T18:57:37Z

biogeochem/EDPatchDynamicsMod.F90

             canopy_mortality_root_litter(p) = canopy_mortality_root_litter(p) + &
-                  canopy_dead*(currentCohort%br+currentCohort%bstore)
+                  canopy_dead*(fnrt_c + store_c)


@rgknox FYI use of "dead" here - justing pointing you to it

thanks @serbinsh , in this case, dead really refers to dead trees, specifically the number density of dead trees

serbinsh · 2018-10-18T18:58:46Z

biogeochem/EDPhysiologyMod.F90

+
+          leaf_c   = currentCohort%prt%GetState(leaf_organ, all_carbon_species)
+
+          currentCohort%treelai = tree_lai(leaf_c, currentCohort%pft, currentCohort%c_area, &


sorry just continuing this thread, here leaf_c in the calc of LAI for each cohort.....

serbinsh · 2018-10-18T18:59:42Z

biogeochem/EDPhysiologyMod.F90

@@ -366,6 +410,12 @@ subroutine phenology( currentSite, bc_in )
    integer  :: day                      ! day of month (1, ..., 31)
    integer  :: sec                      ! seconds of the day

+    real(r8) :: leaf_c                       ! leaf carbon [kg]


minor, comment alignment is off (my OCD is showing)

You know you can suggest edits with the new button for these things?

I couldnt figure out how to "align" it with the other comments in that block otherwise I would have done that ;)

ok, will change

serbinsh · 2018-10-18T19:01:44Z

biogeochem/EDPhysiologyMod.F90

-                   currentCohort%bl          = 0.0_r8                                        
+
+                   ! Remember what the lai (leaf mass actually) was for next year
+                   currentCohort%laimemory   = leaf_c


Huh? "next" year? Also why do we consider leaf mass also and area? Personally I think that is confusing

I guess technically it should be leafc_memory... Do we still need this, @rgknox, in the presence of the new flushing parameter and the target-centric allocation scheme. It might be better, although not bfb, to have leaf flushing as a fraction of the target and forget about lai_memory?

I am fine with that @rosiealice . It seems a little cleaner to me, but I didn't want to mess with existing stuff. If it has your blessing I will remove laimemory and just use the target.

serbinsh · 2018-10-18T19:03:36Z

biogeochem/FatesAllometryMod.F90

@@ -580,7 +580,10 @@ real(r8) function tree_lai( bl, pft, c_area, nplant, cl, canopy_lai)
    !----------------------------------------------------------------------

    if( bl  <  0._r8 .or. pft  ==  0 ) then
-       write(fates_log(),*) 'problem in treelai',bl,pft


Is that a sufficient warning/error message? Idea is to then go to that portion of the output?

I am not a fan of this error message either @serbinsh . In fact, it triggers quite a bit, because machine math precision will generate small negatives that are effectively zero, like 1e-19. But it will complain. I think this comparison should be against, not zero, but a some number close to zero but higher than machine precision, like our carbon allocation error tolerance. If it triggers, it should fail, and be more verbose.

Actually, I just realized I had deleted the warning.

i will remove the if clause (although compilers will remove it anyway I think)

…leaf_c

…ile now.

rgknox · 2018-10-19T18:10:09Z

FYI, github is not updating the code changes I make to rectify many of your comments. Or at least the updates are not appearing "inline" on this thread. The git side of things (the branch itself) seems to updating correctly though.

… Also added flexibility in leaf flushing algorithm.

…al, fixed day-> conversion factor

rgknox · 2018-10-24T18:49:33Z

Performed a final comparison with master. 100 year simulation, BCI, spin-up from bare ground, default 2 tropical clones.

acre report here: https://drive.google.com/file/d/1B2Asdo9obQZ0WvOWDbkZ7dlQ-dp-858z/view?usp=sharing

I compared their timing information, the parteh version was about 3% slower. I was expecting it to be slower by a small margin, I suppose that is about what 3% is. The flexibility that the code offers in connecting arbitrary hypotheses to the rest of the code comes at a marginal performance cost.

Incidentally, @jkshuman and I have both not experienced noticable performance hits on global gridded runs.

rgknox · 2018-10-24T18:51:14Z

We have a few other PR's submitted right now that are fairly straight forward.
I will integrate those that can be done without deliberations or checks. Then I will integrate PARTEH. For those PRs that do not get integrated before parteh, I will help with the conflict resolutions.

rgknox · 2018-10-24T19:42:44Z

I went through the list of PRs. Only one other PR is ready for testing, and the changes are quite small, so I will integrate parteh now.

rgknox and others added 23 commits September 18, 2018 15:19

Added parteh into FATES folder system. Functional unit tests for carb…

857d873

…on pass. Not hooked into FATES though.

Partial first pass on integrating new modular allocation system

229dce0

Lots more conversion from old system to parteh v0

9140338

Minor syntax changes to describing main prt object

3aafcae

minor syntax updates to prt generic

e7fedaa

Converted biophysics routines into new allocation structure. First pass.

8b58a68

Removed old CN parameters

a3b68cb

Added the call to load the PRT instances

39fbed2

First pass at code handling loss fluxes, updates to parameters.

f987da9

Updating calls to loss-fluxes. Removed the cohort litter_flux variabl…

d3f760f

…e (redundant).

Code fixes and tweaks to enable new turnover design into the parteh u…

f6b47fd

…nit testing framework.

Updated functional unit test build script to include external CNP module

22b62f8

Resolved merge conflicts with master.

efe38c2

Bug fixes to maintenance losses, fixes/updates to the unit test to us…

9a34945

…e the CNP hypothesis (external).

Added phenology events to the functional unit tests, however its mess…

fb39f3c

…y and needs cleanup.

Debugged phenology events in the unit testing framework

b27fbee

Lots of debugging of parteh integration. This changeset compiles.

a40884f

Debugging parteh v1, model runs, crashes after several years due to c…

11c4855

…arbon inbalance.

More debugging for parteh-fates. Added reproductive flux handler.

d1497bb

bug fixes relevant to parteh v0. Fixed root litter accumulation and f…

ec51801

…usion weighting

Enabled object oriented filling/retrieving of PRT variables from rest…

33c8fa9

…art. Untested

Some debugging for restarts on arbitrary prt variables.

dcb9219

bug fixes in parteh v1

f133b79

rgknox added enhancement PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. labels Oct 3, 2018

rgknox mentioned this pull request Oct 3, 2018

parameter naming conventions for new allocation and transport parameters #422

Closed

rgknox and others added 2 commits October 3, 2018 16:49

Fixed some descriptions in the parameter file. Added the flush fracti…

19f634d

…on parameter, and checks.

Fixed type definition of prt objects during copy and fuse

43dcde8

serbinsh reviewed Oct 18, 2018

View reviewed changes

ckoven and others added 8 commits October 18, 2018 14:30

Update parteh/PRTGenericMod.F90

9bf1ad7

Update parteh/PRTGenericMod.F90

ba3a24e

Update parteh/PRTLossFluxesMod.F90

a2119ad

Update parteh/PRTAllometricCarbonMod.F90

ee39364

Update biogeochem/EDPatchDynamicsMod.F90

ad301bc

More updates to comments and variable naming conventions, like bl -> …

ef87e48

…leaf_c

unified the grperc parameters, only grperc is used in the parameter f…

a2dd80d

…ile now.

Updated functional unit tests to conform to newly named variables.

4be98cc

rgknox added 2 commits October 22, 2018 13:37

parteh: Changed naming convention from species (chemical) to element.…

daacfb2

… Also added flexibility in leaf flushing algorithm.

parteh: Updated species (chem) to element in functional unit test code

f20cd68

rgknox added PR status: Ready This PR is ready for integration after having passed through peer review and final testing. and removed PR status: Not Ready The author is signaling that this PR is a work in progress and not ready for integration. labels Oct 22, 2018

parteh: placed call to net_allocation query inside an isnew condition…

c8d2025

…al, fixed day-> conversion factor

rgknox merged commit 5a9b1d6 into NGEET:master Oct 24, 2018

rgknox deleted the rgknox-parteh branch October 31, 2023 18:57


		leaf_c = currentCohort%prt%GetState(leaf_organ, all_carbon_species)

		currentCohort%treelai = tree_lai(leaf_c, currentCohort%pft, currentCohort%c_area, &

parteh v1 #421

parteh v1 #421

Conversation

rgknox commented Oct 3, 2018 • edited Loading

Description:

Collaborators:

Expectation of Answer Changes:

Checklist:

Test Results:

rgknox commented Oct 3, 2018

rgknox commented Oct 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgknox commented Oct 19, 2018

rgknox commented Oct 24, 2018

rgknox commented Oct 24, 2018

rgknox commented Oct 24, 2018

rgknox commented Oct 3, 2018 •

edited

Loading