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

nutrient dynamics v2 of allocation and acquisition #880

Merged
merged 61 commits into from
Dec 2, 2022

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented Jun 17, 2022

Description:

This set of changes brings in version two of nutrient dynamics acquisition and allocation. Specifically, this algorithm evaluates the relative state of carbon and nutrient stores in the plant, and uses that to allocate more or less fine-roots. Fine root biomass increases and decreases act as a proxy for activity, and therefore respiration costs.

This will be paired with a ctsm and e3sm pull request. The e3sm code is written but no PR. The ctsm side code is not written. I expect that code refactors to how FATES interacts with CTSM BGC will be implemented either prior to this, or in concert with this PR.

This branch of E3SM is compatible with this work, and will be organized into a PR as this one makes progress:
https://github.com/rgknox/E3SM/tree/rgknox/lnd/update-suppl-fates-uptake-api24

This PR with CTSM should be synchronized, and is an API change: ESCOMP/CTSM#1874

Collaborators:

@ckoven Xinyuan Wei @walkeranthonyp Jinyun Tang Qing Zhu @jenniferholm @dmricciuto Xiaojuan Yang

Expectation of Answer Changes:

This will change answers in nutrient limited simulations, ie parteh mode 2

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:

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

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

FATES baseline hash-tag:

Test Output:

rgknox and others added 26 commits October 18, 2021 14:39
…gulation would not occur at nutrient storage fractions less than the growth cutoff.
…lants to hold overflow storage in the CNP version, which prevents dumping to soil which would otherwise promote decomposition from reducing mineralized pools. Also, changed storage nutrients to not key off of target root biomass, which was creating an instability in the dynamic root code.
…:rgknox/fates into cnp-dynamic-root-allom-dfdd-api21-symfix
… that tracks which speciens (CNP) limits growth
@rgknox rgknox added enhancement parameter file Pertaining to changes to the FATES parameter file 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 19, 2022
@rgknox
Copy link
Contributor Author

rgknox commented Oct 19, 2022

There's two features in this PR related to history that should be pointed out:

  1. A new history dimension was added "CLSZPF". This is canopy layer x size-class x functional-type. I added to to track the L2FR variable, which seems to vary by all three axes. Example of its definition is here: https://github.com/NGEET/fates/pull/880/files#diff-03feca0f1df8d3791c0d8f7b6055a0368d5241716d19d6a97c3d227fd6c5ad23R5350-R5354

  2. A new history routine was added, update_history_nutrflux(). This updates history output for nutrient fluxes, prior to disturbance, which prevents the need to remember these fluxes and rescale them when patch areas change. Personally I like to see us expand this type of thing. There are other examples of fluxes that we save in a special array just for its diagnostic, to prevent its scaling when patch areas change, when the easier/simpler solution is to just update the history arrays themselves prior to disturbance. Here is the routine: https://github.com/NGEET/fates/pull/880/files#diff-03feca0f1df8d3791c0d8f7b6055a0368d5241716d19d6a97c3d227fd6c5ad23R1747 and here is the call (during ed_integrate_state_variables ) https://github.com/NGEET/fates/pull/880/files#diff-873439f6e1a0575bdcd8784eb63e6f1c65347c75a6598e8d169e40c2e8fb2090R553-R556

Copy link

@walkeranthonyp walkeranthonyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic Ryan, great job. A few comments related to RD demand calculation and the /c costs of Nfix. Plus a couple re output variable naming conventions.

! If the plant is not a newly recruited plant
! We use other methods of specifying nutrient demand
! -----------------------------------------------------------------------------------

if(element_id.eq.nitrogen_element) then

plant_demand = smth_fac*ccohort%daily_n_demand + (1._r8-smth_fac)*max(0._r8,ccohort%daily_n_need)
plant_demand = fnrt_c * EDPftvarcon_inst%vmax_nh4(ccohort%pft) * sec_per_day

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come plant_demand for N is calculated using only the Vmax for NH4. Wouldn't it make sense conceptually to include the Vmax for NO3 as well? ... That said, for the same Vmax values the RD simulations already take up a lot more N than the ECA simulations (in the FACE site simulations anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. RD wants the plants to present a joint unified demand on nitrogen uptake. It does this because the demand is used to drive NH4 uptake, and then the remainder is applied to NO3. Thus with only one demand, we use only one parameter vmax_nh4.

However, we don't have to do it this way. ECA needs the vmax differentiated for NH4 and NO3. It might be cleaner if we have the vmax parameters the same for both RD and ECA. But, in the case of RD, we will simply add them together...?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was expecting to see the sum of NO3 and NH4 Vmax multiplied by the root mass to get the whole plant demand. Just based on the concept.

That would increase plant competitiveness versus microbes in RD. I'm not sure what that balance looks like currently. But with RD in the FACE simulations, plants take up equivalent levels of N to ECA at much lower Vmax values, at least an order of magnitude lower Vmax if not more. That makes sense as RD and ECA are fundamentally different approaches, it's unlikely that with ECA N uptake is operating near Vmax for much of the time.

So while conceptually it makes sense to add them, it will further increase plant competitiveness and further widen the gap between RD and ECA uptake magnitudes for the same Vmax values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the meaning of the vmax parameters for RD are much different than with ECA. Right now there is just no way around it, because RD applied the same demand sequentially to the NH4 and then to the NO3. Whereas ECA applies the two different demands NH4 and NO3 separately and exclusively to their parameters.

After this PR, we can re-evaluate how we form the NO3 demand from FATES to RD. We could apply the demands in RD independantly. But I would like to do it as a separate thing because it would require some non-trivial coding, particularly because that code assumes there is only 1 joint n demand term.

biogeophys/FatesPlantRespPhotosynthMod.F90 Show resolved Hide resolved
! This is the unit carbon cost for nitrogen fixation. It is temperature dependant [kgC/kgN]
c_cost_nfix = s_fix * (exp(a_fix + b_fix * (t_soil-tfrz) &
* (1._r8 - 0.5_r8 * (t_soil-tfrz) / c_fix)) - 2._r8)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this temp-driven variation in C costs of N fix come from, is it a pub?

My concern here is that it might over-estimate C costs at higher temp. Here's my line of reasoning. The C cost here is the cost of doing work, i.e. metabolic N fixation. The rate of that metabolism is increased by temperature, so respiration increases but so should N fixation shouldn't it? So the cost of the work per unit N fixation doesn't necessarily increase, but it's the overall metabolic rate that increases so that both C loss and N fixation increases per unit time. Does that make sense?

Rather than a strong evidence base, this comes more from a general theoretical hunch I have that in model world we too often think of respiration only as a cost and don't consider the benefit of whatever process is using that energy. Yes, T increases respiration but that represents a general increase in metabolic rates.

Happy to discuss further and look at the evidence but I can't find that Houlton et al 2010 paper referenced for the parameters in this function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ckoven -- ahhh, I see. I hadn't looked closely at the form of the function. I'd assumed it was some relative of Q10 or Arrhenius but this seems much more reasonable.

real(r8), allocatable :: eca_km_nh4(:) ! half-saturation constant for plant nh4 uptake [gN/m3]
real(r8), allocatable :: eca_vmax_nh4(:) ! maximum production rate for plant nh4 uptake [gN/gC/s]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it just the vmax_nh4 variable that's had the eca_ prefix removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is because the vmax_nh4 parameter is used for both eca and rd, while the vmax_no3 parameter is just used for eca. Again, it might be better to use both to define demand for RD, as a sum.

main/EDTypesMod.F90 Outdated Show resolved Hide resolved
avgflag='A', vtype=site_pft_r8, hlms='CLM:ALM', upfreq=1, &
ivar=ivar, initialize=initialize_variables, index = ih_recl2fr_canopy_pf)

call this%set_history_var(vname='FATES_RECL2FR_USTORY_PF', units='kg kg-1', &

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the convention is here but I would encourage and underscore in between REC and L2FR. Thinking about it, I also would encourage a more bigendian type approach going from the broadest category to the more finer detailed, so putting REC after USTORY, so FATES_L2FR_USTORY_REC_PF. Above as well.

I guess there are lots of ways you could slice this but definitely encourage the _ no matter what :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might had been worried that the var name was going to get too long and chopped by the host models. But I can at least try adding an underscore to see if it works, it would be nicer.

initialize=initialize_variables, index = ih_ndemand_scpf)

call this%set_history_var(vname='FATES_SYMNFIX_SZPF', units='kg m-2 s-1', &
long='symbiotic dinitrogen fixation, by size-class x pft in kg N per m2 per second', &

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest NFIX_SYM also line 5412 below.

Also for free-living Nfix too. NFIX_FREE or whatever it is, but that way what you see first with both of them is that it's Nfix.

upfreq=1, ivar=ivar, initialize=initialize_variables, &
index = ih_nefflux_si)
call this%set_history_var(vname='FATES_FROOTN_SZPF', units='kg m-2', &
long='fine-root nitrogen mass by size-class x pft in kg N per m2', &

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an observation that fluxes tend to have the element first, e.g. PUPTAKE while stores tend to have the element second, e.g. FROOTN.

@rgknox
Copy link
Contributor Author

rgknox commented Nov 15, 2022

Through discussions with @walkeranthonyp in this thread, I modified some parameters. Now, the potential nutrient uptake rates per unit fineroot biomass parameters, (vmax), have the same parameter names for use in rd and eca. See here:
https://github.com/NGEET/fates/pull/880/files#diff-91857484d2b9a65d2cb5a44ecfc1614fbc9567fede7867365cdc42502faa13f0R1007-R1012

Note also, that RD will now use the sum of the NH4 and NO3 vmax terms. So users who have been using this branch with RD, should halve their NH4 and NO3 parameters (previously it just ignored the NO3 term when using RD).

All changes to the parameter file are encapsualted here: https://github.com/NGEET/fates/blob/fa8841e24a761f5fef6a769e7fe3c63312d7417e/parameter_files/apichange_24to25.xml

@rgknox
Copy link
Contributor Author

rgknox commented Nov 29, 2022

loaded the new parameter file onto cheyenne and imported

fates test suite running: /glade/scratch/rgknox/tests_1128-200653ch comparing with fates-sci.1.60.2_api.24.2.0-ctsm5.1.dev112

@rgknox
Copy link
Contributor Author

rgknox commented Nov 30, 2022

Initial tests are showing differences compared to base in the FATES_LITTER_OUT and FATES_CBALANCE_ERROR, no other differences detected so far. Here is how the fates_litter_out diagnostic is constructed:

https://github.com/NGEET/fates/blob/main/main/FatesHistoryInterfaceMod.F90#L3485-L3490

In this PR, effluxes from the plant (due to excess carbon) are added to the litter fragmentation flux, where in main they were not. This is also reflected in the error term, as they have the exact same difference as the fragmentation flux.
These are typically very small, and from the test results, are very small. Honestly, I forget why I did this (but Ryan from the past may have had a good reason!). I think its because it made it cleaner to track carbon conservation between the host and fates.

@rgknox
Copy link
Contributor Author

rgknox commented Nov 30, 2022

I finished going through the fates test suite results, and confirmed that differences were only associated with the change in the carbon efflux error term, and how that is exported to the litter flux (expected and part of this PR). Expected exeptions to this were in the PRT2 test (which should be different, the module was completely changed) and those tests already flagged with expected failures.

test dir: rgknox/tests_1130-082657ch

@rgknox rgknox merged commit e663a6e into NGEET:main Dec 2, 2022
@rgknox rgknox deleted the cnp-dynamic-root-allom-dfdd-api21-symfix branch October 31, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement parameter file Pertaining to changes to the FATES parameter file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants