Skip to content

Conversation

@mvdebolskiy
Copy link
Contributor

@mvdebolskiy mvdebolskiy commented Jan 26, 2026

Description of changes

As described in #3677,when there is more than enough heat to melt h2osoi_ice extra heat (xm2) was calcuated incorrectly based on the updated h2osoi_ice value.

Specific notes

Will post plots in the next comments

Contributors other than yourself, if any: @changemode @swensosc

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

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

Yes, this will change soil temperature/moisture state (and everything else that depends on it) within ~10 years, starting from current intial conditions. Soil would be warmed less (correctly) in the layer where all soil ice and some excess ice have melted.

Any User Interface Changes (namelist or namelist defaults changes)?
No
Does this create a need to change or add documentation? Did you do so?
No
Testing performed, if any:
Manual tests with I2000ClmBgcCrop + aux_clm on derecho.

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).

@swensosc
Copy link
Contributor

As part of this fix, would it be possible to remove some of the commented-out lines, and also to change the name from 'phasechange_beta' to just 'phasechange'? The '_beta' was added during testing many years ago, and could be removed.

@mvdebolskiy
Copy link
Contributor Author

@swensosc I can clean-up this tomorrow.

Here are some figures for long experiments I've done with I200Clm60BgcCrop on f19_g17 grid. All the difference graphs are done default - this pr. Max abs err means maximum of the absolute difference between default and this PR for the whole period given monthly output. forcXX means how many degrees there have been added to forc_tbot_grc for the experiment.

This is for year 29 month 2 where maximum difference in soil temperature at 3 m is observed. The point plot is the gridcell with the largest absolute error for 3 m temperature.
maps_maxt3_f0
point_maxt3_f0

Same as above, but adding +50 degrees to the forcing. Maximum absolute difference is now on year 14.
maps_maxt3_f5
point_maxt3_f5

Following figures show difference for the end of simulation. The point ploted is the the one with maximum difference for excess ice volume fraction by the end of 40 year run with 0 degrees added to forcing.
maps_maxviend_f0
point_maxviend_f0

Last figure is the maximum differences for: Subsidence, Excess ice volume, total soil ice specific mass and mean column soil temperature.

maps_aberr_f0

@mvdebolskiy
Copy link
Contributor Author

All in all, the fix does not create huge differences in the soil state in the long run, however, there are some odd gridcells, I can look at other variables, probably latent heat flux/NBP/runoff? But I doubt that after the spinup from coldstart f.e. there will be drastically different states.

Aux_clm gives baseline diffs, but all the tests are quite short (longest, I think is 3y?) so the differences are tiny, but in a lot of variables (which is expected)

@wwieder
Copy link
Contributor

wwieder commented Jan 27, 2026

Thanks for making these plots, @mvdebolskiy. How does permafrost depth (ALTMAX) change with the bug fix?

@ekluzek ekluzek changed the title Fix for xm2 and revise logic for excess ice melt ctsm5.4.014: Fix for xm2 and revise logic for excess ice melt Jan 27, 2026
@ekluzek ekluzek self-assigned this Jan 27, 2026
@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations science Enhancement to or bug impacting science non-bfb Changes answers (incl. adding tests) labels Jan 27, 2026
@ekluzek ekluzek added this to the cesm3_0_beta08 milestone Jan 27, 2026
@github-project-automation github-project-automation bot moved this to Ready to start (or start again) in CTSM: Upcoming tags Jan 27, 2026
@ekluzek ekluzek moved this from Ready to start (or start again) to In progress - master in CTSM: Upcoming tags Jan 27, 2026
@ekluzek
Copy link
Collaborator

ekluzek commented Jan 27, 2026

@mvdebolskiy the tag that comes before this is about done. So I'm going to start working on bringing this in. So I'll push updates to master and the like. Let me know if anything comes up in your testing that shows I shouldn't proceed forward with it.

@mvdebolskiy
Copy link
Contributor Author

@wwieder
Here are mean abs error and max abs error for the runs:
Only few gridcells seem to have large values.
maps_avaberr_f0_aux
maps_maaberr_f0_aux
maps_avaberr_f5_aux
maps_maaberr_f5_aux

Here are spatial averages of mean abs error and max abs error:

forc 0  ALTMAX  meanmean meanmax:    0.0011    0.0436

forc 0  EFLX_LH_TOT  meanmean meanmax:    0.0020    0.1197

forc 0  QFLX_EVAP_TOT  meanmean meanmax:   1.0e-07 *    0.0079    0.4679

forc 0  QOVER  meanmean meanmax:   1.0e-06 *    0.0017    0.1444

forc 0  QRUNOFF  meanmean meanmax:   1.0e-06 *    0.0025    0.1753

forc 0  TOTECOSYSC  meanmean meanmax:    0.0987    0.4361

forc 0  TWS  meanmean meanmax:    0.0522    0.5514

forc 0  FCH4  meanmean meanmax:   1.0e-10 *    0.0017    0.1445

forc 0  FCO2  meanmean meanmax:   1.0e-09 *    0.0067    0.2785

forc 5  ALTMAX  meanmean meanmax:    0.0020    0.0344

forc 5  EFLX_LH_TOT  meanmean meanmax:    0.0022    0.1131

forc 5  QFLX_EVAP_TOT  meanmean meanmax:   1.0e-07 *    0.0087    0.4510

forc 5  QOVER  meanmean meanmax:   1.0e-07 *    0.0062    0.5585

forc 5  QRUNOFF  meanmean meanmax:   1.0e-07 *    0.0119    0.7994

forc 5  TOTECOSYSC  meanmean meanmax:    0.1085    0.2951

forc 5  TWS  meanmean meanmax:    0.0384    0.4828

forc 5  FCH4  meanmean meanmax:   1.0e-12 *    0.0196    0.8766

forc 5  FCO2  meanmean meanmax:   1.0e-09 *    0.0045    0.2184

@mvdebolskiy
Copy link
Contributor Author

@ekluzek I've updated up to master:HEAD. You can review this and test.

Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. @swensosc approved the science changes. There several things that make the code easier to read as well:

  • Drop the _beta suffix from the subroutine name which didn't seem to have a meaning
  • Remove commented out code that made it confusing
  • Change a setting of a value to a "if else if" structure that makes it easier to compare the difference in the if (better way to code and better for performance)
  • A few key comments that help

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 28, 2026

@mvdebolskiy awesome this is great. I'll do the testing and bring this in shortly.

Two questions for you:

First -- do you want to create the ChangeLog or -- should I based on the comments here?
Second -- since the simulations you ran were used for a tag, I want to have the details in LMWG_dev. Should I do that -- or do you want to?

@swensosc
Copy link
Contributor

one small addition: could we add a longer explanation here?
instead of:
xm2(c,j) = xm(c,j) - wice0(c,j) ! Leftover melt
use this:
! If xm > wice0, then all soil ice melts, and the remaining heat (xm2) is used to melt excess ice
xm2(c,j) = xm(c,j) - wice0(c,j)

@mvdebolskiy
Copy link
Contributor Author

mvdebolskiy commented Jan 28, 2026

@ekluzek
I've addressed @swensosc's comments above and added a draft for Changelog.

I want to have the details in LMWG_dev

You want me to make an issue there?

Copy link
Contributor

@swensosc swensosc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 28, 2026

@mvdebolskiy

I want to have the details in LMWG_dev

You want me to make an issue there?

Yes. Add an issue there, and fill out everything that makes sense in the template.

You probably ran on a machine in Norway, and likely didn't run the LDF or ILAMB. But, document what you have, and maybe display whatever plots you have -- or maybe point to the plots here? For the LDF/ILAMB results they have a website where they put data that they can just point to. If you do have something like that it would be good to have -- but don't worry about it if you don't. Also since the case data is in Norway, you just report about where it is and the casenames and such. We don't need to copy it anywhere, just report it like it is. Since, we've only used LMWG_dev for cases on Derecho, you might need to adjust some of the templates. But, it will be a good test case to document a case ran in Norway, so that we can do this for other cases in the future. It helps highlight that CTSM is run around the world and highlights the NorESM/CTSM partnership.

@mvdebolskiy
Copy link
Contributor Author

I've ran those cases on derecho and looked at the output with matlab on my local machine. I will post an issue tomorrow.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 29, 2026

Testing is as expected on Derecho and Izumi so far. Testing is complete on Derecho and there's only one test left on Izumi.

Only the following 22 tests showed differences to answers. They are all clm6_0 physics which would be expected, and mostly longer tests. Most of the shorter tests, must not have shown differences (and the Izumi tests are all short, so it wasn't shown there either).

ERP_D_Ld5.ne30pg3_t232.IHistClm60Sp.derecho_intel.clm-default--clm-nofireemis
ERP_Ly3_P64x2.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-cropMonthOutput--clm-matrixcnOn_ignore_warnings
ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_gnu.clm-monthly
ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly            EXPECTED POSSIBILITY
ERP_P64x2_Ld396.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings            EXPECTED POSSIBILITY
ERP_P64x2_Ld765.f10_f10_mg37.I2000Clm60BgcCrop.derecho_intel.clm-monthly
ERS_Ld396.f10_f10_mg37.I1850Clm60Bgc.derecho_intel.clm-monthly_matrixcn_fast_spinup
ERS_Ly3.f10_f10_mg37.I1850Clm60BgcCropCmip6.derecho_intel.clm-basic
ERS_Ly3.f10_f10_mg37.I2000Clm60BgcCrop.derecho_intel
ERS_Ly5_P128x1.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-cropMonthOutput
ERS_Ly5_P128x1.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-cropMonthOutput--clm-matrixcnOn_ignore_warnings
ERS_P128x1_Ld765.f10_f10_mg37.I2000Clm60Fates.derecho_intel.clm-FatesColdNoComp
LCISO_Ld396.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-ciso_monthly
LCISO_Ld396.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-ciso_monthly--clm-matrixcnOn_ignore_warnings
RXCROPMATURITYSKIPGEN_Ld1097.f10_f10_mg37.IHistClm60BgcCrop.derecho_intel.clm-cropMonthOutput
SEQ_D_Ld10_PS.f09_f09_mt232.I1850Clm60Sp.derecho_intel.clm-default--clm-nofireemis
SMS.ne30pg3_t232.I2000Clm60BgcCrop.derecho_intel.clm-clm60cam7LndTuningMode
SMS_D_Ld10.f09_f09_mt232.IHistClm60BgcCrop.derecho_intel.clm-f09_FillMissingW_Urban
SMS_D_Ld5.f09_g17.ISSP245Clm60BgcCropCrujra.derecho_intel.clm-default
SMS_Ly2_PS.f19_g17.I2000Clm60BgcCrop.derecho_intel.clm-cropMonthOutput
SMS_Ly3.f10_f10_mt232.IHistClm60BgcCropCrujra.derecho_intel.clm-ciso_cmip7_monthly_2013Start
SMS_Ly3.f10_f10_mt232.IHistClm60BgcCropCrujra.derecho_intel.clm-ciso_monthly_2013Start

@ekluzek ekluzek merged commit 7871ea9 into ESCOMP:master Jan 29, 2026
4 checks passed
@github-project-automation github-project-automation bot moved this from In progress - master to Done (non release/external) in CTSM: Upcoming tags Jan 29, 2026
@ekluzek ekluzek deleted the pr-fix-xm2-excess-ice branch January 29, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement new capability or improved behavior of existing capability non-bfb Changes answers (incl. adding tests) priority: high High priority to fix/merge soon, e.g., because it is a problem in important configurations science Enhancement to or bug impacting science

Projects

Status: Done (non release/external)
Status: Done

Development

Successfully merging this pull request may close these issues.

Logic error in excess ice melt calculation

4 participants