Skip to content

Conversation

@eclare108213
Copy link
Contributor

  • Short (1 sentence) summary of your PR:
    Enforce a minimum snow grain radius to prevent NaNs.
  • Developer(s):
    @njeffery
  • Suggest PR reviewers from list in the column to the right.
  • Please copy the PR test results link or provide a summary of testing completed below.

Results differ for all tests using snwgrain = .true. in base_suite:

224 measured results of 224 total results
219 of 224 tests PASSED
0 of 224 tests PENDING
0 of 224 tests MISSING data
5 of 224 tests FAILED
(base) Mac-mini:testsuite.snowradius eclare$ ./results.csh | grep FAIL
FAIL conda_macos_smoke_col_1x1_bgcispol_debug compare base20260127 different-data
FAIL conda_macos_smoke_col_1x1_debug_run1year_snw30percent_snwgrain compare base20260127 different-data
FAIL conda_macos_restart_col_1x1_bgcispol compare base20260127 different-data
FAIL conda_macos_restart_col_1x1_snwgrain_snwitdrdg_zaero compare base20260127 different-data
FAIL conda_macos_restart_col_1x1_snwgrain_snwitdrdg compare base20260127 different-data

  • How much do the PR code changes differ from the unmodified code?
    • bit for bit
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on CICE or any other models?
    • Yes
    • No
  • Does this PR add any new test cases?
    • Yes
    • No
  • Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
    • Yes
    • No, does the documentation need to be updated at a later time?
      • Yes
      • No
  • Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
    If the snow grain radius is set to zero, possibly because of zapping small ice or if ice disappears mid-timestep, then updates of snow grain radius will produce NaNs. Snow grain radius is usually bounded between a min and max so this generally doesn't happen, but a recent coupled E3SM bgc run crashed with this error. While the error seems to be relatively rare, this bug fix changes answers when the snow grain radius is nonzero but still less than the minimum.

@apcraig
Copy link
Contributor

apcraig commented Jan 27, 2026

I'm testing in CICE now.

@apcraig
Copy link
Contributor

apcraig commented Jan 28, 2026

Testing looks good on Derecho. I am getting some answer changes, I think that is expected. All snow cases except the snw30percent cases and all bgc cases are not bit-for-bit. All other cases are bit-for-bit.

I will approve this change and merge in the morning unless I hear otherwise. I will then create a new CICE PR with the updated Icepack hash and will move forward with release testing.

Copy link
Contributor

@njeffery njeffery left a comment

Choose a reason for hiding this comment

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

I've tested this bugfix in a short (4-year) GCASE simulation in E3SM and in a fully-coupled simulation that's run 10 years. The latter simulation crashed in update_snow_radius without the fix indicating that the snow grain radius was zero.

@apcraig apcraig merged commit daa4163 into CICE-Consortium:main Jan 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants