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

"use_cn" is used inconsistently in soilbiogechem for FATES than in the rest of the model #942

Open
ekluzek opened this issue Mar 12, 2020 · 4 comments
Assignees
Labels
code health improving internal code structure to make easier to maintain (sustainability)

Comments

@ekluzek
Copy link
Collaborator

ekluzek commented Mar 12, 2020

For most of the model the logical namelist variable "use_cn" is used as a flag to say that the above ground Carbon-Nitrogen model is turned on. However, when FATES is on use_cn=.false. is used in the soil biogeochemistry to signal that Nitrogen isn't in use.

Here's a grep of the code for soil-BGC:

SoilBiogeochemLittVertTranspMod.F90:  use clm_varctl                         , only : iulog, use_c13, use_c14, spinup_state, use_vertsoilc, use_fates, use_cn
SoilBiogeochemLittVertTranspMod.F90:            if (use_cn ) then
SoilBiogeochemPrecisionControlMod.F90:    use clm_varctl , only : iulog, use_c13, use_c14, use_nitrif_denitrif, use_cn
SoilBiogeochemPrecisionControlMod.F90:                  if (use_cn) then
SoilBiogeochemPrecisionControlMod.F90:            if (use_cn) then
SoilBiogeochemStateType.F90:  use clm_varctl     , only : use_vertsoilc, use_cn 
SoilBiogeochemStateType.F90:    if (use_cn) then

The thin that's done in LittVertTransfer is...

      !------ loop over litter/som types
      do i_type = 1, ntype

         select case (i_type)
         case (1)  ! C
            conc_ptr          => soilbiogeochem_carbonstate_inst%decomp_cpools_vr_col
            source            => soilbiogeochem_carbonflux_inst%decomp_cpools_sourcesink_col
            trcr_tendency_ptr => soilbiogeochem_carbonflux_inst%decomp_cpools_transport_tendency_col
         case (2)  ! N
            if (use_cn ) then
               conc_ptr          => soilbiogeochem_nitrogenstate_inst%decomp_npools_vr_col
               source            => soilbiogeochem_nitrogenflux_inst%decomp_npools_sourcesink_col
               trcr_tendency_ptr => soilbiogeochem_nitrogenflux_inst%decomp_npools_transport_tendency_col
            endif

So it's basically used to tell soil-BGC that FATES isn't using Nitrogen.

This is first of all confusing to use it for two purposes like this. Second, FATES is going to be adding nutrients in via PARTEH and one of the first nutrients will be Nitrogen. So there should be a different flag that can be used to tell soil-BGC that Nitrogen is on or off.

In above ground BGC the supplemental Nitrogen flag supl_nitro is used to say that Nitrogen is unlimited. But, that flag is currently incompatible with the new CLM5.0 Nitrogen options, so it's use should be limited.

As such, I think soil-BGC should have a different flag that's set internal to the model to determine if soil-BGC is using Nitrogen or no. Maybe something like soilbgc_nitro? It probably shouldn't be a namelist item, because it isn't something that a user would toggle on or off.

@rgknox @glemieux @bishtgautam

@ekluzek ekluzek added the code health improving internal code structure to make easier to maintain (sustainability) label Mar 12, 2020
@ekluzek ekluzek self-assigned this Mar 12, 2020
@rgknox
Copy link
Collaborator

rgknox commented Mar 12, 2020

Thanks for starting this thread @ekluzek

I agree, use_cn does seem to be more about above-ground than below ground. And as you point out, when FATES with nutrients is active, it will want to interact with potentially the same set of below-ground code that cn vegetation would. So, imo, use_cn should not be used to describe non-vegetation processes.

@ekluzek
Copy link
Collaborator Author

ekluzek commented Apr 22, 2021

A document that @rgknox and @wwieder and I came up with to sort this out is here:

https://docs.google.com/spreadsheets/d/1wwI_RuQ87FNk4PdrSLtlplisEMTRqDCxFeqG6Wbxt_c

@wwieder
Copy link
Contributor

wwieder commented Apr 22, 2021 via email

@ekluzek
Copy link
Collaborator Author

ekluzek commented Dec 31, 2023

Note, also the discussion here:

https://github.com/ESCOMP/CTSM/pull/2076/files/93a18c82177577d73234337688b185f8095f2467#r1361283876

we should decide at what level use_cn should be used and not use it beneath that level. It's used inconsistently at both high levels and low levels. If the high level logic is correct it doesn't need to be done at the lower level, unless the purpose is for error checking for debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health improving internal code structure to make easier to maintain (sustainability)
Projects
None yet
Development

No branches or pull requests

3 participants