Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potentially incorrect allocation to fine roots in PRTAllometricCarbonMod.F90? #784

Open
mpaiao opened this issue Sep 29, 2021 · 7 comments

Comments

@mpaiao
Copy link
Contributor

mpaiao commented Sep 29, 2021

I was looking at this code and it seems to me that allocation to fine roots would be underestimated:

if( (carbon_balance > nearzero ) .and. (total_c_demand>nearzero)) then
leaf_c_flux = min(leaf_c_demand, &
carbon_balance*(leaf_c_demand/total_c_demand))
carbon_balance = carbon_balance - leaf_c_flux
leaf_c(iexp_leaf) = leaf_c(iexp_leaf) + leaf_c_flux
fnrt_c_flux = min(fnrt_c_demand, &
carbon_balance*(fnrt_c_demand/total_c_demand))
carbon_balance = carbon_balance - fnrt_c_flux
fnrt_c = fnrt_c + fnrt_c_flux
end if

Carbon balance is subtracted after finding leaf_c_flux, and then scaled by fnrt_c_demand/total_c_demand, so the relative demand of fine root demand is applied in a lower carbon balance value. Shouldn't the code be something like the one below?

       leaf_c_flux    = min(leaf_c_demand, &
                        carbon_balance*(leaf_c_demand/total_c_demand))
       fnrt_c_flux    = min(fnrt_c_demand, &
                            carbon_balance*(fnrt_c_demand/total_c_demand))
       leaf_c(iexp_leaf) = leaf_c(iexp_leaf) + leaf_c_flux
       fnrt_c         = fnrt_c + fnrt_c_flux
       carbon_balance = carbon_balance - leaf_c_flux - fnrt_c_flux
@mpaiao
Copy link
Contributor Author

mpaiao commented Sep 29, 2021

And I think the same rationale applies to the block a few lines above.

if (total_c_demand> nearzero ) then
! We pay this even if we don't have the carbon
! Just don't pay so much carbon that storage+carbon_balance can't pay for it
leaf_c_flux = min(leaf_c_demand, &
max(0.0_r8,(store_c+carbon_balance)* &
(leaf_c_demand/total_c_demand)))
! Add carbon to the youngest age pool (i.e iexp_leaf = index 1)
carbon_balance = carbon_balance - leaf_c_flux
leaf_c(iexp_leaf) = leaf_c(iexp_leaf) + leaf_c_flux
! If we are testing b4b, then we pay this even if we don't have the carbon
fnrt_c_flux = min(fnrt_c_demand, &
max(0.0_r8, (store_c+carbon_balance)* &
(fnrt_c_demand/total_c_demand)))
carbon_balance = carbon_balance - fnrt_c_flux
fnrt_c = fnrt_c + fnrt_c_flux
end if

@rgknox
Copy link
Contributor

rgknox commented Sep 29, 2021

I agree @mpaiao

@rgknox
Copy link
Contributor

rgknox commented Sep 29, 2021

yeah, we should not be updating the carbon balance until both pools have calculated their fluxes

@jkshuman
Copy link
Contributor

I also agree with you @mpaiao good catch.

mpaiao added a commit to mpaiao/fates that referenced this issue Oct 18, 2021
1. Implemented option to drive leaves on / leaves off using soil matric potential instead
   of soil water content, which is more intuitive for setting up thresholds. I left the
   original setting for back compatibility and added a flag in the parameter files to
   switch between SWC and SMP thresholds (fates_phen_drought_refvar) and a separate
   parameter to define the SMP threshold (fates_phen_smp_threshold).
2. When drought-deciduous leaves are off, the plant now stops allocating carbon to any
   living tissue, to reduce carbon losses when carbon balance is necessarily non-positive.
3. Bug fix in DailyPRTAllometricCarbon (parteh/PRTAllometricCarbonMod.F90). When
   allocating to different tissues, the code was subtracting allocation of tissues before
   calculating the amount for the next tissue, potentially under-allocating carbon to fine
   roots. See issue NGEET#784.
4. Implemented an option carbon allocation routine (DailyPRTAllometricCarbonSimpler in
   parteh/PRTAllometricCarbonMod.F90).  In this routine, I applied much simpler rules for
   maintenance allocation: it simply checks how much each pool is in deficit, and allocates
   carbon (storage + carbon_balance) to the pools according to the debt. If there is any
   carbon left, then the plant finishes filling the storage pool and then allocates to
   growth.
@mpaiao
Copy link
Contributor Author

mpaiao commented Oct 18, 2021

I changed the code to set the fluxes of all pools before changing carbon balance. You can check the changes in my local commit: mpaiao@bdd5efc

I should just note that this causes a substantial difference in the dynamics. In my tests for the semi-arid site in Brazil, just switching this order caused evergreens to go nearly extinct during an extreme drought year (not necessarily a problem because I was running a tropical moist forest evergreen in a semi-arid site).

@mpaiao
Copy link
Contributor Author

mpaiao commented Oct 18, 2021

I also had the impression that the carbon allocation routine is rather complex. I wrote a simplified version that simply allocates the available carbon (storage + carbon balance) towards all pools, proportionally to the deficit of each pool, and if there is any remaining carbon then it allocates to growth. I put this subroutine as an option in PRTAllometricCarbonMod.F90 (link below)

https://github.com/mpaiao/fates/blob/bdd5efc377be5f681ce0266fad423787771741b8/parteh/PRTAllometricCarbonMod.F90#L928

Do you think this approach makes sense?

@rgknox
Copy link
Contributor

rgknox commented Oct 18, 2021

I like simple, seems fine to have the alternative.
In the N & P enabled allocation scheme, we allow the user to define the order in which replace tissues. We could potentially merge our ideas and create a c-only version:
https://github.com/NGEET/fates/blob/master/parteh/PRTAllometricCNPMod.F90#L615

mpaiao added a commit to mpaiao/fates that referenced this issue Oct 30, 2021
1. Bug fix in DailyPRTAllometricCarbon (parteh/PRTAllometricCarbonMod.F90). When
   allocating to different tissues, the code was subtracting allocation of tissues before
   calculating the amount for the next tissue, potentially under-allocating carbon to fine
   roots.
2. Renamed variable laimemory with leafmemory. We were never tracking the LAI, but leaf
   carbon, and thus the name is dangerously misleading.
3. Implemented an option carbon allocation routine (DailyPRTAllometricCarbonSimpler in
   parteh/PRTAllometricCarbonMod.F90).  In this routine, I applied much simpler rules for
   maintenance allocation: it simply checks how much each pool is in deficit, and allocates
   carbon (storage + carbon_balance) to the pools according to the debt. If there is any
   carbon left, then the plant finishes filling the storage pool and then allocates to
   growth.
@glemieux glemieux linked a pull request Nov 5, 2021 that will close this issue
5 tasks
mpaiao added a commit to mpaiao/fates that referenced this issue May 3, 2022
…ots and leaves are truly

proportional to demand.

Description:

1. Bug fix in DailyPRTAllometricCarbon (parteh/PRTAllometricCarbonMod.F90). When allocating
to different tissues, the code was subtracting allocation of tissues before calculating
the amount for the next tissue, potentially under-allocating carbon to fine roots.
2. Minor code updates to simplify allocation calculations. It now uses a single
allocation factor based on availability and total need, which is applied to all tissues.
3.  Replaced a few if statements with select case, which simplifies adding other hypotheses
in the future (and it's safer for selecting cases).

This pull request addresses the bug discussed in NGEET#784 and supersedes pull request NGEET#800.
Additional minor changes in former PR NGEET#800 will be added as subsequent pull requests.
@mpaiao mpaiao mentioned this issue May 4, 2022
5 tasks
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 a pull request may close this issue.

3 participants