Move growth respiration to daily timestep #1197

ckoven · 2024-05-10T04:13:38Z

This PR moves the calculation of growth respiration (GR) to the daily timestep, as a tax on daily integrated max(0,(GPP - MR)), instead of half-hourly max(0,(GPP-MR)). This will lead to a decrease in growth respiration because it includes the nighttime respiration in the loss term.

Description:

By moving the GR to daily, it means that NPP is also only defined at the daily timestep as well. With one exception, I moved all history variables that depend on growth respiration to also be daily. The one exception was I kept NEP as a high frequency output for the case where a user wants to compare directly to NEE data; however this requires (1) making the assumption that GR is constant on timescales less than a day, and (2) using a given cohort's GR from the prior day (which thus introduces some slight error since cohort n will have changed since the GR was actually applied).

fixes #1194
potentially:
fixes #1229

Collaborators:

Discussed on 4/29 FATES SW call, and further discussions with @JessicaNeedham and @rgknox.

Expectation of Answer Changes:

This will be answer-changing.

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

The in-code documentation has been updated with descriptive comments
The documentation has been assessed to determine if updates are necessary

Integrator

FATES PASS/FAIL regression tests were run
Evaluation of test results for answer changes was performed and results provided

Documentation

Technical Note update:
User's Guide update:

Test Results:

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

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

FATES baseline hash-tag:

Test Output:

…-hourly to daily

rgknox · 2024-05-17T15:50:10Z

biogeochem/FatesCohortMod.F90

+    real(r8) :: resp_m_acc
+    real(r8) :: resp_m_acc_hold
+
+    real(r8) :: resp_g_acc                ! Growth respication can only be calculated at the daily timestep


@ckoven , do we need resp_g_acc? The _acc variable was created to track the accumulation of the flux through the fast timesteps over the course of the day. Since we don't accumulate growth respiration anymore, since it is defined only at the end of the day, I believe we only need resp_g_acc_hold, right?

right, this was a thing I wasn't sure about. I was guessing that it is as you describe, but wanted to include the _acc variable to be on the safe side. If you don't think it is needed, then happy to remove.

rgknox · 2024-05-17T15:53:46Z

biogeophys/EDAccumulateFluxesMod.F90

@@ -82,15 +82,13 @@ subroutine AccumulateFluxes_ED(nsites, sites, bc_in, bc_out, dt_time)

                   if ( debug ) then


we should remove this debug statement. we can add these types of write statements in when needed, but not maintained in the main codebase

rgknox · 2024-05-17T15:55:02Z

biogeophys/FatesPlantRespPhotosynthMod.F90

@@ -987,23 +987,15 @@ subroutine FatesPlantRespPhotosynthDrive (nsites, sites,bc_in,bc_out,dtime)

                        ! convert from kgC/indiv/s to kgC/indiv/timestep
                        currentCohort%resp_m        = currentCohort%resp_m  * dtime
+                        currentCohort%resp_m_tstep  = currentCohort%resp_m   ! these two things are the same. we should get rid of one them


agreed, I vote we get rid of resp_m since resp_m_tstep is more descriptive

rgknox · 2024-05-17T16:02:53Z

main/FatesHistoryInterfaceMod.F90

@@ -2814,6 +2815,15 @@ subroutine update_history_dyn1(this,nc,nsites,sites,bc_in)
               ! have any meaning, otherwise they are just inialization values
               notnew: if( .not.(ccohort%isnew) ) then



As part of an upcoming refactor, we will be adding calls to daily fluxes that occur before we change plant number densities. As already acknowledged, the number of plants present in this point of the call sequence is not the same number of plants that generated the growth respiration flux. This is a known issue, but it outside the scope of this PR.

Here is the last place in the dynamics call sequence where number density has not changed yet: https://github.com/NGEET/fates/blob/sci.1.76.3_api.35.1.0/main/EDMainMod.F90#L751

Here is where we make a call to write daily nutrient fluxes, which also happens before we change number density: https://github.com/NGEET/fates/blob/sci.1.76.3_api.35.1.0/main/EDMainMod.F90#L698

rgknox · 2024-06-12T17:32:31Z

This may or may not fit into this PR @ckoven, but I did want to point out that we have another carbon flux that is triggered daily. When the nutrient model is active, if carbon is not the limiting resource, any carbon that was available for growth but was not allocated to tissues and storage will be respired off. The variable is cohort%resp_excess, and it is resolved at the end of the calls to growth, here:

https://github.com/NGEET/fates/blob/main/main/EDMainMod.F90#L595

Ideally, the plant's dynamic root allocation schemes should nudge the plant's resource allocation balance so that it is not wasting resources, but nonetheless this is a term that should be accounted for if anyone is tracking NEE or other carbon flux balancing while the nutrient model is active.

JessicaNeedham

Thanks @ckoven ! This mostly looks good to me. It compiled after I fixed a few very minor things.

JessicaNeedham · 2024-06-27T19:32:18Z

main/EDMainMod.F90

+
+             ! at this point we have the info we need to calculate growth respiration
+             ! as a "tax" on the difference between daily GPP and daily maintenance respiration
+             if (hlm_use_ed_prescribed_phys .eq. itrue) then


Is this if else statement the wrong way around? if prescribed physiology is true then we should set resp_g_acc to 0 as in the comments?

JessicaNeedham · 2024-06-27T20:28:15Z

main/FatesHistoryInterfaceMod.F90

@@ -2377,6 +2375,9 @@ subroutine update_history_dyn1(this,nc,nsites,sites,bc_in)
         hio_sum_fuel_si         => this%hvars(ih_sum_fuel_si)%r81d,  &
         hio_litter_in_si        => this%hvars(ih_litter_in_si)%r81d, &
         hio_litter_out_si       => this%hvars(ih_litter_out_si)%r81d, &
+         hio_npp_si              => this%hvars(ih_npp_si)%r82d, &


should be r81d

JessicaNeedham · 2024-06-27T20:37:01Z

main/EDMainMod.F90

There's a resp_acc on line 675 that needs to be updated

JessicaNeedham · 2024-06-27T20:48:39Z

biogeophys/FatesPlantRespPhotosynthMod.F90

@@ -735,7 +735,7 @@ subroutine FatesPlantRespPhotosynthDrive (nsites, sites,bc_in,bc_out,dtime)

                           ! Zero cohort flux accumulators.


This npp_tstep needs to be deleted now.

JessicaNeedham · 2024-06-29T03:02:22Z

Results from a global nocomp simulation with the default parameter file. Growth respiration decreased by about 10% with this change. This is just the mean over a 10 year simulation from bare ground so definitely not spun up yet.

rgknox · 2024-07-01T14:27:38Z

@JessicaNeedham , that 10% decrease seems about what I would had expected, do you see it the same way?

JessicaNeedham · 2024-07-01T16:41:29Z

@JessicaNeedham , that 10% decrease seems about what I would had expected, do you see it the same way?

I think so

main/EDMainMod.F90

main/FatesHistoryInterfaceMod.F90

biogeophys/FatesPlantRespPhotosynthMod.F90

Co-authored-by: Jessica Needham <10586303+JessicaNeedham@users.noreply.github.com>

biogeophys/FatesPlantRespPhotosynthMod.F90

… gpp does. Fixed some output units. Fixed carbon flux tracking when we bypass (ie st3 and satphen).

…files

Growth respiration

rgknox · 2024-11-01T16:00:17Z

I believe this PR is content complete and has had a thorough examination.

glemieux · 2024-11-12T23:37:54Z

Initial regression testing underway on derecho.

glemieux · 2024-11-13T00:54:53Z

Testing on derecho against fates-sci.1.79.2_api.36.1.0-ctsm5.3.008 shows no unexpected test failures. As expected all fates test mods result in DIFFs.

Spot checking the FatesColdAllVars differences shows what I believe is expected behavior with the following variables being the only differences in the first fates dynamics timestep (i.e. t_index = 2):

FATES_AUTORESP
FATES_AUTORESP_CANOPY
FATES_GROWTH_RESP
FATES_MAINT_RESP_UNREDUCED
FATES_NEP
FATES_NPP
FATES_NPP_AP
FATES_GROWAR_CANOPY_SZ
FATES_AUTORESP_SZPF
FATES_GROWAR_SZPF

Beyond this timestep, more differences show up in additional variables. The differences for the variables above are on the order of E-09.

Results: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr1197

glemieux · 2024-11-13T17:46:12Z

Checking the FatesColdSatPhen tests, the only diffs I'm seeing are with FATES_MAINT_RESP_UNREDUCED which are around E-08.

ckoven added 3 commits May 7, 2024 14:44

first set of changes to move growth respiration calculation from half…

8104707

…-hourly to daily

starting to modify history vars

d03b44f

more edits to history fields

2df2832

ckoven added the draft label May 10, 2024

rgknox reviewed May 17, 2024

View reviewed changes

rgknox requested a review from JessicaNeedham June 24, 2024 19:16

JessicaNeedham reviewed Jun 27, 2024

View reviewed changes

JessicaNeedham reviewed Jul 1, 2024

View reviewed changes

main/EDMainMod.F90 Outdated Show resolved Hide resolved

JessicaNeedham reviewed Jul 1, 2024

View reviewed changes

main/FatesHistoryInterfaceMod.F90 Outdated Show resolved Hide resolved

Update main/FatesHistoryInterfaceMod.F90

b6cad14

JessicaNeedham reviewed Jul 1, 2024

View reviewed changes

biogeophys/FatesPlantRespPhotosynthMod.F90 Outdated Show resolved Hide resolved

glemieux self-assigned this Aug 22, 2024

JessicaNeedham approved these changes Oct 14, 2024

View reviewed changes

Apply suggestions from code review

9080ba3

Co-authored-by: Jessica Needham <10586303+JessicaNeedham@users.noreply.github.com>

rgknox reviewed Oct 30, 2024

View reviewed changes

biogeophys/FatesPlantRespPhotosynthMod.F90 Outdated Show resolved Hide resolved

rgknox added 4 commits October 30, 2024 13:27

Added respiration excess to sub-daily npp tracking by holding it like…

a0dea3b

… gpp does. Fixed some output units. Fixed carbon flux tracking when we bypass (ie st3 and satphen).

removing resp_g_acc

f5a7caa

removing resp_g_acc, it is unnecessary

1d52034

merge resolution between growth_respiration and main

366b73a

rgknox and others added 4 commits October 30, 2024 22:32

various bug fixes for growth respiration update

3f688d7

Added hard-coded override to revert to original format for inventory …

a9758b3

…files

syntax updates and minor bug fix for respiration update

50bf6b9

Merge pull request #24 from rgknox/growth_respiration

9d28eb6

Growth respiration

rgknox removed the draft label Nov 1, 2024

rgknox self-requested a review November 1, 2024 15:58

rgknox approved these changes Nov 1, 2024

View reviewed changes

glemieux merged commit de5f266 into NGEET:main Nov 14, 2024
1 check passed

This was referenced Jan 4, 2025

Landuse fixes history interface deconflict with the latest tag ckoven/fates#26

Merged

Bug fix for izumi nag tests to pass (PR into tmp-241219) ESCOMP/CTSM#2925

Merged

Land Use Fixes #1273

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move growth respiration to daily timestep #1197

Move growth respiration to daily timestep #1197

ckoven commented May 10, 2024 •

edited by glemieux

Loading

rgknox May 17, 2024

ckoven May 17, 2024

rgknox May 17, 2024

rgknox May 17, 2024

rgknox May 17, 2024 •

edited

Loading

rgknox May 17, 2024

rgknox commented Jun 12, 2024

JessicaNeedham left a comment

JessicaNeedham Jun 27, 2024

JessicaNeedham Jun 27, 2024

JessicaNeedham Jun 27, 2024

JessicaNeedham Jun 27, 2024

JessicaNeedham commented Jun 29, 2024

rgknox commented Jul 1, 2024

JessicaNeedham commented Jul 1, 2024

rgknox commented Nov 1, 2024

glemieux commented Nov 12, 2024

glemieux commented Nov 13, 2024

glemieux commented Nov 13, 2024

		@@ -82,15 +82,13 @@ subroutine AccumulateFluxes_ED(nsites, sites, bc_in, bc_out, dt_time)

		if ( debug ) then

		@@ -2814,6 +2815,15 @@ subroutine update_history_dyn1(this,nc,nsites,sites,bc_in)
		! have any meaning, otherwise they are just inialization values
		notnew: if( .not.(ccohort%isnew) ) then

		@@ -735,7 +735,7 @@ subroutine FatesPlantRespPhotosynthDrive (nsites, sites,bc_in,bc_out,dtime)

		! Zero cohort flux accumulators.

Move growth respiration to daily timestep #1197

Move growth respiration to daily timestep #1197

Conversation

ckoven commented May 10, 2024 • edited by glemieux Loading

Description:

Collaborators:

Expectation of Answer Changes:

Checklist

Documentation

Test Results:

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 May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgknox commented Jun 12, 2024

JessicaNeedham left a 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

JessicaNeedham commented Jun 29, 2024

rgknox commented Jul 1, 2024

JessicaNeedham commented Jul 1, 2024

rgknox commented Nov 1, 2024

glemieux commented Nov 12, 2024

glemieux commented Nov 13, 2024

glemieux commented Nov 13, 2024

ckoven commented May 10, 2024 •

edited by glemieux

Loading

rgknox May 17, 2024 •

edited

Loading