Merge-tag to resolve issue #2366

[slevis-lmwg]

Description of changes

#2366 removes an instance of negative snocan in CanopyFluxesMod.F90 to resolve a test failure. Issue #2366 points out that snocan appears negative elsewhere, as well, and that possibly this is ok.

Specific notes

Contributors other than yourself, if any:
@billsacks

CTSM Issues Fixed (include github issue #):
Fixes #2366

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

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

Testing performed, if any:
#2366 shows the LWISO test passing when not permitting negative snocan.
Next I will submit the test-suites.

[slevis-lmwg]

I labeled this PR as a blocker because it needs to be merged for CTSM5.2 testing to pass (specifically the LWISO test).

[olyson]

I'll just note that regarding your comment "Yes, likely more than roundoff for urban cases and roundoff for other cases.", that the t_soisno changes will have effects larger than rounoff for cold start simulations in non-urban landunits, e.g., as seen here:

https://webext.cgd.ucar.edu/I1850/ctsm51d159_f45_GSWP3_bgccrop_tsoiini_1850pSASU/lnd/ctsm51d159_f45_GSWP3_bgccrop_tsoiini_1850pSASU_51_60-ctsm51d159_f45_GSWP3_bgccrop_tsoiini_1850pAD_571_580/setsIndex.html

[olyson]

Did I push the wrong button just now? I added a comment to this, but I just got an email: Auto Assign to Project(s): All jobs have failed

[slevis-lmwg]

No @olyson you're fine. This is a new thing that went in recently that doesn't work, yet.

[slevis-lmwg]

Test-suites
./run_sys_tests -s aux_clm -c ctsm5.1.dev167 -g ctsm5.1.dev168_pr2371
izumi OK
derecho OK

TODO Rename the new baseline when ready to merge.

[slevis-lmwg]

Izumi tests are showing NLCOMP diffs as follows:

<   BASE: paramfile = ctsm51_params.c240207.nc'
<   COMP: paramfile = ctsm51_params.c240207b.nc'

Is this ok?

[slevis-lmwg]

Is this ok?

As far as I can tell, this is ok. These are @samsrabin's param file updates that removed FillValue etc.

[slevis-lmwg]

@samsrabin I think it just means that you generated dev167 baselines with the previous param file version rather than the latest.

[slevis-lmwg]

@olyson
A test in the izumi test-suite has revealed a possible problem with your temperature change. I ran all the combinations/permutations (with + without your code and with + without my code) and found that the test passes when I run without your code, whether mine is in or not.

The derecho test-suite is still queued, so you may have time to figure this out and give me a fix before it starts; however, I'm inclined to revert your mods if that's ok with you.

Here's more info about the test that fails:
SMS_Lm3_D_Mmpi-serial.1x1_brazil.I2000Clm50FatesCruRsGs.izumi_intel.clm-FatesColdHydro

forrtl: error (73): floating divide by zero
Image              PC                Routine            Line        Source       
cesm.exe           00000000047FC6DB  Unknown               Unknown  Unknown
libpthread-2.17.s  00002B74A7C0D630  Unknown               Unknown  Unknown
cesm.exe           00000000047CE001  Unknown               Unknown  Unknown
cesm.exe           0000000003D270A5  fateshydrowtfmod_         771  FatesHydroWTFMod.F90
cesm.exe           0000000001DE07E5  fatesplanthydraul         578  FatesPlantHydraulicsMod.F90
cesm.exe           000000000178CF34  edcohortdynamicsm         264  EDCohortDynamicsMod.F90

[slevis-lmwg]

The derecho test-suite has started now, so let's go with my suggestion of reverting #2355.

[olyson]

Interesting, I guess the fact that it was never implemented properly when it originally came to main meant that it was never fully tested with the test-suite. I guess FATES doesn't like initializing the soil temperature below freezing for some reason. That makes me wonder about what happens when excess_ice is on since it initializes soil temperature to be 5deg below freezing. Regardless, I'll look at it but I would revert the mods because I'm not that familiar with FATES and probably won't be able to come up with a quick solution.

[slevis-lmwg]

One-line change where I disallow negative snocan from this section of code.

[slevis-lmwg]

@ekluzek I'm ready to merge.

[ekluzek]

Just went over this with @slevis-lmwg and this is great. We had suggested some more improvements, but they will take some effort to get right and make sure is working properly. And it's important to get this in for CTSM5.2 right away. So simpler is best.