Bringing the PPE mods to master via b4b-dev

[slevis-lmwg]

Description of changes

Initially bringing the diffs between branch_tags/PPE16... and branch_tags/PPE15... to b4b-dev manually.

TODOs listed in these cards:
https://github.com/orgs/ESCOMP/projects/39/views/1?pane=issue&itemId=73918053 DONE
https://github.com/orgs/ESCOMP/projects/39/views/1?pane=issue&itemId=73918054 DONE

• Also update params files with PPE contents and address The c3psn list in CTSM's params files has four crops set incorrectly #1652 in the process
• ./rimport the new files

Specific notes

Contributors other than yourself, if any:
@linniahawkins @ekluzek @olyson

CTSM Issues Fixed (include github issue #):
Fixes #2567
Fixes #1652

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

Any User Interface Changes (namelist or namelist defaults changes)?
New params files

Does this create a need to change or add documentation? Did you do so?

Testing performed, if any:
Ran aux_clm on branch_tags/PPE16... and branch_tags/PPE15... successfully as documented in #2567

[ekluzek]

Code changes look great. And it looks like you removed commented out code and uneeded comments which is great.

Marking as approved, although as already noted the parameter files need to be updated with the new fields.

[ekluzek]

@linniahawkins this is the one last bit we think we need for the PPE work (in terms of the CTSM code).

[slevis-lmwg]

Thank you @ekluzek

I also asked @olyson for a review in case he has code that I still need to bring in here (based on our discussion yesterday).

[olyson]

I compared these four files against what I had in /glade/work/oleson/PPE.n11_ctsm5.1.dev030_djk2120, which contains the code modifications that @djk2120 and I had. It looks like those modifications agree with what is shown here in this PR.
I do have one question, probably for @djk2120 . In my/Daniels code modifications, there are changes to src/biogeophys/SoilStateInitTimeConstMod.F90 that are not here in this PR or on master. For example, this comment and associated code changes are deleted in my/Daniel's version, but are still in master:

This is separated into sections for non-perturbation and perturbation of sand/clay because the perturbation code is not bfb when sand_pf=clay_pf=0. This occurs because of a divide and then a multiply in the code.

I'm not sure if this was changed later on or not. @djk2120 , do you remember?

[olyson]

Ok, so the SoilStateInitTimeConstMod.F90 code Linnia is currently using (/glade/work/linnia/PPEn14trans) agrees with what's in this PR and in master, so maybe this is ok as is.

[slevis-lmwg]

...and from this morning's email thread I gather that the param file updates that I made are good:

• I ncdump-ed the latest param files (dated c240208)
• I added mods from /glade/campaign/cgd/tss/people/oleson/modify_param/c*_params.c210507_kwo.c220322.nc
• I ncgen-ed new files (dated c240814)

[slevis-lmwg]

I submitted ./run_sys_tests -s aux_clm -c ctsm5.2.015 --skip-generate

• izumi OK (1 test still PEND)
• derecho OK (3 "unexpected" tests that differ from the baseline match the failures reported here

[slevis-lmwg]

Based on the test results, this is ready.
b4b-dev is locked at the moment, so this PR will wait for the next b4b-dev cycle.

[wwieder]

Thanks @slevis-lmwg. If testing hasn't started on the locked b4b branch is there harm in unlocking b4b to bring this in sooner? Not sure if this breaks the spirit of b4b dev or helps us out much, but thought I'd ask

[slevis-lmwg]

Thanks @slevis-lmwg. If testing hasn't started on the locked b4b branch is there harm in unlocking b4b to bring this in sooner? Not sure if this breaks the spirit of b4b dev or helps us out much, but thought I'd ask

Worth asking @wwieder.
I will defer to @samsrabin who is in charge of merging b4b-dev to master this time around.

[samsrabin]

The testing is unfortunately well under way by now. I have one problematic test, though, and if I can't get it to work I might have to delay the b4b-dev merge. I'll keep you updated.

[samsrabin]

Nope, I got it done. b4b-dev merge will continue.

[slevis-lmwg]

This step DONE

./rimport -file lnd/clm2/paramdata/clm45_params.c240814.nc 
./rimport -file lnd/clm2/paramdata/clm50_params.c240814.nc 
./rimport -file lnd/clm2/paramdata/ctsm51_params.c240814.nc 
./rimport -file lnd/clm2/paramdata/ctsm51_ciso_cwd_hr_params.c240814.nc
./rimport -file lnd/clm2/paramdata/ctsm60_params.c240814.nc 
./rimport -file lnd/clm2/paramdata/ctsm60_params_cn30.c240814.nc 

Next step: Merge into b4b-dev.

[slevis-lmwg]

OK ./build-namelist_test.pl: 451/3339 < FAIL> <Test Id: 451> <Desc: LeungDust_WO_Prigent> I saw a thread between @samsrabin and @ekluzek confirming that this is expected to fail, but I can't locate the thread now...

./run_sys_tests -s aux_clm -c ctsm5.2.023 --skip-generate
OK izumi
OK derecho: This is the only unexpected failure I get from ./cs.status.fails | grep -v PASS | grep -v NLCOMP
REP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings BASELINE ctsm5.2.023: DIFF which matches the results here and, therefore, is ok.

[olyson]

Reviewing the original PPE spinup and historical simulation in preparation for a new spinup (issue #70 in LMWG_dev), I'm seeing two code modifications that are not appearing in ctsm5.2.028. These were in the last PPE branch tag (branch_tags/PPE.n16_ctsm5.1.dev030) in CanopyFluxesMod.F90 and CNVegStructUpdateMod.F90, but are not in ctsm5.2.028.

[wwieder]

Thank for checking this, Keith. I'll let others weigh in on the details here, but appreciate your careful eye

[olyson]

I guess my comment above about CanopyFluxesMod.F90 and CNVegStructUpdateMod.F90 doesn't really belong in this PR, since this one deals with the bfb part of the the PPE branch. I'll make my comment in Issue #2567 .