ESCOMP
Removing non doalb call of wrap_canopy_radiation for fates
Description of changes
This reverts a previous change that enabled a call to fates' wrap_canopy_radiation and zenith angle updates when the doalb flag was false. The original implementation was (apparently) needed for elm-fates to enable branch type restart runs, but appears to be unnecessary for clm-fates. The previous code was causing problems for cam-fates coupled runs.
Specific notes
Contributors other than yourself, if any: @mvdebolskiy performed tests, @ekluzek and @mvertens provided consultation
CTSM Issues Fixed (include github issue #): Fixes: #3043
Are answers expected to change (and if so in what way)? no expected changes
Any User Interface Changes (namelist or namelist defaults changes)?
Does this create a need to change or add documentation? Did you do so?
Testing performed, if any: (List what testing you did to show your changes worked as expected) (This can be manual testing or running of the different test suites) (Documentation on system testing is here: https://github.com/ESCOMP/ctsm/wiki/System-Testing-Guide) (aux_clm on derecho for intel/gnu and izumi for intel/gnu/nag/nvhpc is the standard for tags on master)
so far: ERI_Ld10.f45_f45_mg37.I2000Clm50FatesCru.derecho_gnu.clm-FatesColdTwoStream and ERS_Ld3.f19_g17.I2000Clm50FatesCru.derecho_gnu.clm-FatesColdTwoStream
NOTE: Be sure to check your coding style against the standard (https://github.com/ESCOMP/ctsm/wiki/CTSM-coding-guidelines) and review the list of common problems to watch out for (https://github.com/ESCOMP/CTSM/wiki/List-of-common-problems).
I am testing this in coupled cases.
Also, I want to add a testmod for fates that includes DATM namelist changes same as clm6cam7LndTuningMode. But that can be a separate PR.
Also, there is a need to not allow WARNING in the BalanceCheckMod when fates is on, since the solver there has a different tolerance.
Also in clm-fates interface you are passing non-downscaled short-wave, is there specific reason for this?
@mvdebolskiy , non-downscaled shortwave could just be a a legacy issue, ie we've been using it before a downscale version became available, but I'm just guessing. It has been that way since fates was clm-ed. I suppose there is no reason to not use the most downscaled version possible, but would need to look into how the downscaling is done and what is the actual change of scales (or what subgrid variability is this covering) before I could weigh-in .
The downscaling is just for hillslope hydrology, in general case it equals to greedcell one, so if there is a need to have fates compatible with it, than the column level, downscaled sw should be used, same way as column level coszen.
Yes, @mvdebolskiy is right, you should use downscaled...
The downscaling is just for hillslope hydrology, in general case it equals to gridcell one, so if there is a need to have fates compatible with it, than the column level, downscaled sw should be used, same way as column level coszen.
In columns where that don't have downscaling, they'll be identical, but you'll want it for those cases that do use it. So you should just always use the downscaled version.
Downscaling is also done for glaciers -- which doesn't apply to FATES. But, the point is -- you'll want it for any cases where it has been done.
I am testing this in coupled cases. Also, I want to add a testmod for fates that includes DATM namelist changes same as
clm6cam7LndTuningMode. But that can be a separate PR.
I'd actually like to get this new testcase installed in this PR. Partially, so that we can show that the new test breaks without the change, and it passes with it. This should be easy enough to do. I don't think adding this test will slow this effort down enough that it can't be done now.
@ekluzek and @mvdebolskiy can you take a look at the new tests and give feedback corrections? this is not my wheelhouse
@rgknox they look fine, one thing that might be useful is to make one to have two-tream. You can make it full fates, actually, not just no-comp fixed-biogeo, since the late flags should not matter for the purpose of this tests.
Removing the call for update and normalized radiation breaks hybrid/startup with finidat runs. I added the call back, but restricted it only to fates nocomp with twostream and first timestep on the startup run here. (just remove the diagnostic writeout before wrap_sunfrac).
@mvdebolskiy could you post the error for the break? I'm wondering if other fixes that have been implemented, such as the cross-referenced PR above may address the problem
@rgknox
You will get FPE due to scelg%Kb_leaf being 0 or nan in TwoStreamMPLEMod.F90:512. If you run a hybrid case or ERI test at the second timestep if you just remove the call outside the doalb check, since ZenithPrep is not being run on the first timestep but CanopyPrep is running before ZenithPrep on the second and uses this variable while it did not get assigned meaningful value.
@rgknox can you look at these two commits and apply them? https://github.com/NorESMhub/CTSM/pull/138/commits/aa78277f2f4024290f14ed35543df397535ef214 https://github.com/NorESMhub/CTSM/pull/139/commits/16dd9561318a60b5012eb6739369d5000d404bad
This should fix the ERI tests.
@mvdebolskiy your a legend, I'm getting passes in the following test:
ERI_D_Ld9.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode
Should we get this into the integration pipeline soon?
I think this PR is close to ready. Recent changes:
- I merge it up to master
- expanded test coverage per recommendations of SE team about 3 weeks ago
- I tested with the following FATES-side PR: https://github.com/NGEET/fates/pull/1397 (ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode.20250915_093212_ymt1x8), all PASS
Below is the test fail list as of f427877. Note that most of these are either expected, or simply missing bases since the tests are new. The only test fail that I see is the restart compare on the new ERI test (ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode.C.0921-123413de_gnu), and it is just relegated to diffs on the fates radiation error diagnostics:
RMS FATES_NIR_RAD_ERROR 5.3545E+33 NORMALIZED 1.5978E-02
Although, there is a good chance this will be fixed in FATES 1397
Either way, we need to move forward, I added an expected fail for the test. I will note this in an issue and we can work on it if 1397 does not fix the problem. The good part is that judging by the numbers, this does not seem to be impacting scientifically meaningful results in the restart.
921-123413de_gnu: 14 tests
FAIL ERI_D_Ld9.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode NLCOMP
FAIL ERI_D_Ld9.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode BASELINE fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2: ERROR BFAIL baseline directory '/glade/campaign/cgd/tss/ctsm_baselines/fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2/ERI_D_Ld9.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode' does not exist
FAIL ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode NLCOMP
FAIL ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode COMPARE_base_rest (EXPECTED FAILURE)
FAIL ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode BASELINE fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2: ERROR BFAIL baseline directory '/glade/campaign/cgd/tss/ctsm_baselines/fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2/ERI_D_Ld9.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhenCamLndTuningMode' does not exist
FAIL ERS_D_Ld5.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode NLCOMP
FAIL ERS_D_Ld5.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode BASELINE fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2: ERROR BFAIL baseline directory '/glade/campaign/cgd/tss/ctsm_baselines/fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2/ERS_D_Ld5.f45_f45_mg37.I2000Clm60Fates.derecho_gnu.clm-FatesColdCamLndTuningMode' does not exist
FAIL PEM_D_Ld20.5x5_amazon.I2000Clm50FatesRs.derecho_gnu.clm-FatesColdSeedDisp COMPARE_base_modpes (EXPECTED FAILURE)
FAIL SMS_D_Ld3.f09_g17.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdSatPhen_prescribed RUN time=74 (EXPECTED FAILURE)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL SHAREDLIB_BUILD time=124 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL RUN time=101 (UNEXPECTED: expected FAIL)
0921-123413de_int: 40 tests
FAIL ERI_D_Ld20.f10_f10_mg37.I2000Clm50Fates.derecho_intel.clm-FatesCold BASELINE fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2: DIFF
FAIL ERI_Ld60.f45_f45_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-Fates BASELINE fates-sci.1.87.2_api.41.0.0-ctsm5.3.075-v2: DIFF
FAIL ERP_P256x2_Ld30.f45_f45_mg37.I2000Clm60FatesRs.derecho_intel.clm-mimicsFatesCold RUN time=43 (EXPECTED FAILURE)
PEND ERP_P256x2_Ld30.f45_f45_mg37.I2000Clm60FatesRs.derecho_intel.clm-mimicsFatesCold COMPARE_base_rest
PASS ERS_Ld60.f45_f45_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdST3 RUN time=118 (UNEXPECTED: expected FAIL)
FAIL PVT_Lm3.f45_f45_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesLUPFT RUN time=1590 (EXPECTED FAILURE)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO SHAREDLIB_BUILD time=104 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_intel.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO RUN time=1151 (UNEXPECTED: expected FAIL)