Include coarse woody debris (CWD) in heterotrophic respiration (HR)

[slevis-lmwg]

Description of changes

• Removing if-statements limiting history output of HR to non-CWD pools and decomposition.
• Adding cwdhr_col variable to track this heterotrophic respiration the same way that we track lithr_col.

Specific notes

Contributors other than yourself, if any:
@wwieder
@ekluzek

CTSM Issues Fixed (include github issue #):
Fixes #1361
Fixes #1417

Are answers expected to change (and if so in what way)?
Yes.

• Currently inactive history variables that haven't included CWD to date, now will include CWD.
• Introducing history variables CWDC_HR (for C12, C13, C14). These and other CWD fields pertaining to HR will equal zero with default parameter values rf_cwdl*_bgc = 0. They will be greater than zero and other history fields will change when rf_cwdl*_bgc > 0.

Any User Interface Changes (namelist or namelist defaults changes)?
No

Testing performed, if any:
Running cheyenne test-suite. Not done but this far tests are passing.

[slevis-lmwg]

Both test suites OK
One caveat @ekluzek which may be a known problem:

The scripts cs.status.fails and cs.status give an error, so I used the scripts *gnu, *int, *pgi, *nag to see the results.

[slevis-lmwg]

@wwieder I realize that next week is the LMWG, so no pressure on the code review!

Also I was thinking of folding the solution to #1392 into this PR. I will wait to discuss with you first.

[slevis-lmwg]

@ekluzek is suggesting running a test, baseline and new code, with the relevant history fields active.

[slevis-lmwg]

I ran a single test with the relevant history fields active. The test is successful (PASS). Here are some details:

To generate a new baseline, I changed 8 lines with default inactive to active in master; the four in SoilBiogeochemCarbonFluxType.F90 and the four in SoilBiogeochemNitrogenFluxType.F90 corresponding to my mods in my branch. The baseline that I generated had 32 new history fields not found in the original baseline (otherwise bfb).

I changed the same 8 lines in my branch, which resulted in an additional 8 history fields not found in my new baseline (otherwise bfb). The 8 fields are cwd-related as I would have expected:

Could not find match for file1 variable CWD_HR_L2 in file2
Could not find match for file1 variable CWD_HR_L2_vr in file2
Could not find match for file1 variable CWD_HR_L3 in file2
Could not find match for file1 variable CWD_HR_L3_vr in file2
Could not find match for file1 variable SMINN_TO_LITR2N_CWD in file2
Could not find match for file1 variable SMINN_TO_LITR2N_CWD_vr in file2
Could not find match for file1 variable SMINN_TO_LITR3N_CWD in file2
Could not find match for file1 variable SMINN_TO_LITR3N_CWD_vr in file2

@ekluzek was wondering whether we should keep the new fields (40 in total) as active from now on. This is a question for @wwieder .

@wwieder I was also hoping you'd confirm whether my mods seem correct to you in this PR.
Plus we were going to discuss #1392 and whether to fold that into this PR.
These are just reminders. I am happy to wait for a convenient time for you to respond and/or meet w me.

[wwieder]

few comments related to N here, @slevisconsulting that we can discuss if it's helpful.

we need this on the PPE branch, which makes me wonder what changes need to be made to the parallel matrix code for respiration fluxes of CWD. @ekluzek can you advise

[wwieder]

I'm a little stuck wondering what we should do here... I wasn't imagining this flux would be associated with N mineralization or immobilization at this point. Maybe it should remain with the comment that there is none from CWD

[wwieder]

@slevisconsulting I don't think we need these extra history fields turned on by default. We should check that that the simulations are bfb with rf_cwd = 0 (not sure what the parameter is actually called), but then HR fluxes are greater when rf_cwd > 0.

[ekluzek]

@slevisconsulting yes, I agree with @wwieder they shouldn't be default on. But, I do wonder if they should be turned on for some of our testing. You can add them to hist_fincl1 to the user_nl_clm in one of the test mods.

I'm thinking maybe it should go in the cropMonthOutput test mod as Crop can only be on if one of the BGC modes is on (otherwise test mods can be used for both SP or BGC).

[wwieder]

is this ready to go @slevisconsulting if so, can @ekluzek take it to the PPE branch & main?

[slevis-lmwg]

[...] check that that the simulations are bfb with rf_cwd = 0 (not sure what the parameter is actually called), but then HR fluxes are greater when rf_cwd > 0.

With my current mods (as they appear in the PR), the test is bfb with rf_cwdl2_bgc=0 and rf_cwdl3_bgc=0.

However, when I set rf_cwdl2_bgc=0.5 and rf_cwdl3_bgc=0.5, I get a carbon balance error, for example:

column cbalance error    =   3.063005290509873E-006           8
 Latdeg,Londeg=   30.0000000000000        15.0000000000000
 begcb                    =    435.841365763495
 endcb                    =    435.840938319034
 delta store              =  -4.274444609109196E-004
 --- Inputs ---
 gpp                      =   0.000000000000000E+000
 --- Outputs ---
 er                       =   4.243814556363771E-004
 col_fire_closs           =   0.000000000000000E+000
 col_hrv_xsmrpool_to_atm  =   0.000000000000000E+000
 col_xsmrpool_to_atm      =   0.000000000000000E+000
 wood_harvestc            =   0.000000000000000E+000
 grainc_to_cropprodc      =   0.000000000000000E+000
 -1*som_c_leached         =   1.596739547638851E-014

I have not looked into it, yet. Let me know if you have ideas. And @wwieder you mentioned something about the parallel matrix code. Any chance this error relates to your comment?

[ekluzek]

@slevisconsulting and @wwieder don't worry about the parallel matrix code for now. That's the work on the PPE branch that we might need to provide for when it's moved over to there. Let's get this version working and onto master. When I move it to the PPE branch, I might need some help applying the same changes for the matrix, but possibly not.

Also @slevisconsulting we should add a test mod for this test and add it to the testlist. We'll want to make sure this feature is working for future changes (and for example on the PPE branch). The testmod can point to the params file with the changes you had to make to test it out.

[wwieder]

sorry, bouncing between meetings w/o much time, but did we ever include CWD_HR in the balance check equation? Presumably this needs to be included in the heterotrophic respiration (HR) flux?

[slevis-lmwg]

[...] did we ever include CWD_HR in the balance check equation? Presumably this needs to be included in the heterotrophic respiration (HR) flux?

This is what I'm finding. In fact there is no cwdhr_col variable the way there is lithr_col and somhr_col.

(I'm sorry if this was already known and I should have understood it when reading #1361 )

[slevis-lmwg]

I will add cwdhr_col using lithr_col as template.

[slevis-lmwg]

[...] check that that the simulations are bfb with rf_cwd = 0 (not sure what the parameter is actually called), but then HR fluxes are greater when rf_cwd > 0.

• My latest commit fixes the C balance error.
• I have confirmed in a single test that simulations remain bfb when rf_cwdl*_bgc=0 and that the new history variable CWDC_HR = 0 everywhere.
• I have confirmed using the same test that answers change when rf_cwdl*_bgc=0.5 and that CWDC_HR > 0.

To finish this work, I need to address @ekluzek 's recommendation:

add a test mod for this test and add it to the testlist. We'll want to make sure this feature is working for future changes (and for example on the PPE branch). The testmod can point to the params file with the changes you had to make to test it out.

[wwieder]

one more naive question here @slevisconsulting are there any additional changes that need to be included for summing CWDC_HR fluxes related to isotopes, or is this taken care of?

If so, let's see what @ekluzek says about testing.

[slevis-lmwg]

I discussed the new test with @ekluzek, and we converged on this new test (updated according to later post):
ERS_Ld3.f10_f10_mg37.I2000Clm51Bgc.cheyenne_intel.clm-ciso_cwd_hr
where the new testmod ciso_cwd_hr will add to the ciso testmod the line hist_fincl1 = 'CWDC_HR','C13_CWDC_HR','C14_CWDC_HR','CWD_HR_L2','CWD_HR_L2_vr','CWD_HR_L3','CWD_HR_L3_vr'
and will point to a params file with rf_cwdl*_bgc=0.5

@wwieder I haven't done anything with isotopes in this PR, so I will take a look.

[slevis-lmwg]

...oh, unless you mean C12 vs. C13 vs. C14. I did take care of those.
@wwieder pls let me know if I'm confused.

[ekluzek]

@slevisconsulting if this effects C13 and C14, your test case likely should turn them on as well. There are some Carbon isotope test mods like "ciso", you might want to have your testmod inherit from it, and then keep ciso in the name. So something like ciso_cwd_hr...

[wwieder]

yes, let's make sure the carbon isotopes C13 & C14 tests pass as expected (and are tracking the CWD_HR).

[slevis-lmwg]

yes, let's make sure the carbon isotopes C13 & C14 tests pass as expected (and are tracking the CWD_HR).

I ran the new test ERS_Ld3.f10_f10_mg37.I2000Clm51Bgc.cheyenne_intel.clm-ciso_cwd_hr in two ways:

1. pointing to the default params file where rf_cwd* = 0
2. pointing to my alternate params file where rf_cwd* = 0.5
   In the first case the CWD fields associated with HR (incl. C13 & C14) were zero everywhere.
   In the second case the same fields contained values greater than zero.

I have run the cheyenne and izumi test-suites and they are OK.
I have run ./rimport to get the new params file in the repo.

@wwieder @ekluzek
after a final review of the PR, this work is ready for merge.
Pls let me know of any concerns.
Also let me know when I'm next in line, so that I may rerun the test-suites.

[wwieder]

no additional suggestions here. looks good to me!