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

pconv should be set to 1 for crops #1262

Closed
billsacks opened this issue Jan 22, 2021 · 2 comments · Fixed by #1254
Closed

pconv should be set to 1 for crops #1262

billsacks opened this issue Jan 22, 2021 · 2 comments · Fixed by #1254
Labels
blocker another issue/PR depends on this one bug something is working incorrectly

Comments

@billsacks
Copy link
Member

Brief summary of bug

Crops currently set pconv = pprod10 = pprod100 = 0. This means that, when a crop patch decreases in area, any deadstem in the decreasing area is lost from the system, leading to gridcell carbon balance errors.

General bug information

CTSM version you are using: master, with small mods to trigger the error

Does this bug cause significantly incorrect results in the model's science? No (just slightly incorrect)

Configurations affected: Transient crop cases. It's not clear whether this has any impact in standard configurations, or if it currently only impacts the matrixcn code.

Details of bug

@olyson observed gridcell carbon balance errors in the matrixcn branch.

I reproduced the error from master, with: ERP_D_Ld10_P36x2.f10_f10_musgs.IHistClm51BgcCrop.cheyenne_intel.clm-decStart.

with these diffs:

diff --git a/src/biogeochem/CNPrecisionControlMod.F90 b/src/biogeochem/CNPrecisionControlMod.F90
index 2617dce9a..b49cc9450 100644
--- a/src/biogeochem/CNPrecisionControlMod.F90
+++ b/src/biogeochem/CNPrecisionControlMod.F90
@@ -411,20 +411,6 @@ contains
                                   __LINE__)
       end if
 
-      ! deadstem C and N
-      call TruncateCandNStates( bounds, filter_soilp, num_soilp, cs%deadstemc_patch(bounds%begp:bounds%endp), &
-                                ns%deadstemn_patch(bounds%begp:bounds%endp), pc(bounds%begp:), pn(bounds%begp:), __LINE__, &
-                                num_truncatep, filter_truncatep)
-      if (use_c13) then
-         call TruncateAdditional( bounds, num_truncatep, filter_truncatep, &
-                                  c13cs%deadstemc_patch(bounds%begp:bounds%endp), pc13(bounds%begp:bounds%endp), &
-                                  __LINE__)
-      end if
-      if (use_c14) then
-         call TruncateAdditional( bounds, num_truncatep, filter_truncatep, &
-                                  c14cs%deadstemc_patch(bounds%begp:bounds%endp), pc14(bounds%begp:bounds%endp), &
-                                  __LINE__)
-      end if
       ! deadstem storage C and N
       call TruncateCandNStates( bounds, filter_soilp, num_soilp, cs%deadstemc_storage_patch(bounds%begp:bounds%endp), &
                                 ns%deadstemn_storage_patch(bounds%begp:bounds%endp), pc(bounds%begp:), pn(bounds%begp:), &

where I got:

6: gridcell cbalance error =  1.157233557903814E-007          54
6: nstep =          97
6: latdeg, londeg          =  -10.0000000000000        330.000000000000
6: begcb                   =   19625.6360439932
6: endcb                   =   19625.6048322379
6: delta store             = -3.121175527485320E-002
6: --- Inputs ---
6: nbp_grc                 = -3.376582173493063E-002
6: dwt_seedc_to_leaf_grc   =  2.554182161413064E-003
6: dwt_seedc_to_deadstem_grc =  2.202832180626027E-011
6: --- Outputs ---
6: -1*som_c_leached_grc    =  -8.164843258167253E-015

@olyson determined that the small non-zero deadstemc and deadstemn are being created by these fluxes only:

m_livestemc_to_deadstemc_fire
m_livestemn_to_deadstemn_fire

in both the matrix ON and OFF cases.

The issue is that, for crops, pconv, pprod10, and pprod100 are all 0. These pft parameters are used to determine the fate of deadstem; for non-prognostic crops, there is a check that ensures that they all add to 1, but that check isn't done for crop. I guess there was an assumption that crops won't have deadstem, so it was okay for these to all be 0 for crop. But as @olyson found (noted above), we have non-zero dead stem C for some crop PFTs. I'm not sure if that is correct or not, but what it means is that, when crop area shrinks, the dead stem C & N just disappear into the ether, rather than being divided fully amongst the conversion flux and the 10 and 100 year product pools.

I put this diff in:

diff --git a/src/main/pftconMod.F90 b/src/main/pftconMod.F90
index 88e596505..61024f5d8 100644
--- a/src/main/pftconMod.F90
+++ b/src/main/pftconMod.F90
@@ -1241,10 +1241,13 @@ contains
           end if
           if ( (i /= noveg) .and. (i < npcropmin) .and. &
                abs(this%pconv(i) + this%pprod10(i) + this%pprod100(i) - 1.0_r8) > 1.e-7_r8 )then
              call endrun(msg=' ERROR: pconv+pprod10+pprod100 do NOT sum to one.'//errMsg(sourcefile, __LINE__))
           end if
+          if (i >= npcropmin) then
+             this%pconv(i) = 1._r8
+          end if
           if ( this%pprodharv10(i) > 1.0_r8 .or. this%pprodharv10(i) < 0.0_r8 )then
              call endrun(msg=' ERROR: pprodharv10 outside of range.'//errMsg(sourcefile, __LINE__))
           end if
           if (i < npcropmin .and. this%biofuel_harvfrac(i) /= 0._r8) then
              call endrun(msg=' ERROR: biofuel_harvfrac non-zero for a non-prognostic crop PFT.'//&

and it brought the C balance error down to ~ 1e-12 for the offending grid cell.

@billsacks billsacks added the bug something is working incorrectly label Jan 22, 2021
@billsacks
Copy link
Member Author

As discussed at yesterday's ctsm-software meeting, the solution is to set pconv to 1 for all prognostic crops. I would suggest doing the same for bare ground for completeness. Then this error check in pftconMod:

          if ( (i /= noveg) .and. (i < npcropmin) .and. &
               abs(this%pconv(i) + this%pprod10(i) + this%pprod100(i) - 1.0_r8) > 1.e-7_r8 )then
             call endrun(msg=' ERROR: pconv+pprod10+pprod100 do NOT sum to one.'//errMsg(sourcefile, __LINE__))
          end if

should be generalized to apply to all PFTs.

@billsacks
Copy link
Member Author

Blocks #1263

@billsacks billsacks added the blocker another issue/PR depends on this one label Jan 22, 2021
@ekluzek ekluzek mentioned this issue Jan 22, 2021
ekluzek added a commit to swensosc/ctsm that referenced this issue Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker another issue/PR depends on this one bug something is working incorrectly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant