Advanced snow physics

[eclare108213]

• Short (1 sentence) summary of your PR:
  Icepack implementation of advanced snow physics, ported from E3SM and earlier CICE column versions

• Developer(s):
  E. Hunke, N. Jeffery

• 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.
  https://github.com/CICE-Consortium/Test-Results/wiki/94bf11b34d.badger.intel.21-05-28.221914.0
  The failures are optimization issues - see comments below for additional details.

• How much do the PR code changes differ from the unmodified code?

  • bit for bit when snow mods are turned off
  • 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 provide any additional information or relevant details below:

• Snow redistribution by wind and bulk formulations

• Dry and wet snow grain metamorphosis

• Snow modifications linked back to radiation physics through dEdd

Dry metamorphosis requires a lookup table. The original table (from SNICAR) is netcdf, so there is a subsampled table implemented in Icepack (hardwired) for testing purposes. The full table will be available in CICE.

[eclare108213]

base_suite tests:
https://github.com/CICE-Consortium/Test-Results/wiki/94bf11b34d.badger.intel.21-05-28.221914.0

161 measured results of 161 total results
154 of 161 tests PASSED
0 of 161 tests PENDING
3 of 161 tests MISSING data
4 of 161 tests FAILED
[eclare@ba-fe1 testsuite.ip2]$ results.csh | grep FAIL
FAIL badger_intel_restart_col_1x1_bgcISPOL compare ip1 different-data
FAIL badger_intel_restart_col_1x1_zsal compare ip1 different-data
FAIL badger_intel_restart_col_1x1_thermo1 compare ip1 different-data
FAIL badger_intel_restart_col_1x1_alt04 compare ip1 different-data

3 new tests are missing data for the regression comparison.
The four failures are optimization issues - additional tests were run for them with -O0, which all passed:

20 measured results of 20 total results
20 of 20 tests PASSED
0 of 20 tests PENDING
0 of 20 tests MISSING data
0 of 20 tests FAILED

In the therm1 test, I traced the issue to a new conditional in icepack_meltpond_lvl.F90.
The original code

        ! add melt water
        dvn = rfrac/rhofresh*(meltt*rhoi &
            +                 melts*rhos &
            +                 frain*  dt)*aicen

The modified code

        ! add melt water
        if (use_smliq_pnd) then
           dvn = rfrac/rhofresh*(meltt*rhoi &
               +                 meltsliqn)*aicen
        else
           dvn = rfrac/rhofresh*(meltt*rhoi &
               +                 melts*rhos &
               +                 frain*  dt)*aicen
        endif

Commenting out the dvn = line for use_smliq_pnd = .true. causes the results to be identical when use_smliq_pnd = .false. I suspect similar issues for the other cases.

I believe this PR is ready to merge. However I prefer that we wait until I finish the implementation in CICE, in order to have more thorough testing (including QC).

[eclare108213]

A few comments on recent changes:

The ice and liquid tracers in snow, smice and smliq, are densities, so multiplying them by snow volume gives mass of the ice and liquid components of the snow. Since mass is conserved, it is the most appropriate quantity to work with for the physical changes. However the vertical thermodynamics has enthalpy (i.e. energy density) calculations mixed in, making the associated mass calculations less straight-forward. The vertical thermo code ought to be refactored and cleaned up at some point, but it will change answers and be a huge headache. So not right now.

smice and smliq were not originally consistent with the snow volume on which they were transported. This caused the values of smice and smliq to vary wildly outside of physical bounds during transport, e.g. they could be negative or orders of magnitude larger than the density of water. I tried a number of approaches for maintaining that consistency, finally settling on a relatively compute-intensive approach that enforces mass conservation throughout (and does a better job with monotonicity).

I've also fixed some problems with the smice and smliq reinitializations after snow disappears and comes back, which were causing incorrect answers near the ice edge, e.g. when ice with snow is advected into a grid cell without snow. The test to identify this problem was setting tr_snow=T but turning off all of the new snow processes (redist = none, snwgrain=F), and making sure that the nonzero snow tracers smice and rsnw maintain their constant values everywhere.

A couple of issues when running Icepack and CICE with tr_snow=F were fixed. Thanks @apcraig for running multi-compiler tests to find these problems. One issue that is likely to come up again is using slices of the tracer array in subroutine argument call lists. Sending in tracers with multiple indices (e.g. smice on snow layers), which might not be allocated when the tracer flag (e.g. tr_snow) is false, can cause array-out-of-bounds issues. Maybe we've just been lucky with the configurations that we test. If we don't already do it, we need a test that turns off ALL of the nonessential tracers and checks array bounds. The other issue is that the new snow tests use nslyr=5, and so they can not restart from nslyr=1 initial conditions.

Still testing.

[apcraig]

Why were icepack_warnings_aborted calls removed from icepack_therm_mushy?

I believe answers will change even for cases where snow is off. Any concerns or additional checks needed?

[eclare108213]

The warning changes to icepack_therm_mushy came in with commit 846058d and I figured you had a good reason. :)

I'd like to do another full test suite with regression, since quite a bit has changed since this PR was initially submitted.

[apcraig]

Right, should have looked a little more carefully. The icepack_warnings_aborted were removed because there were extra calls. You can see just below where the calls were removed are the same calls. Sorry about the confusion.

[eclare108213]

The latest base_suite test results:
https://github.com/CICE-Consortium/Test-Results/wiki/065425f318.badger.intel.21-08-05.222435.0

165 measured results of 165 total results
158 of 165 tests PASSED
0 of 165 tests PENDING
3 of 165 tests MISSING data
4 of 165 tests FAILED

MISS badger_intel_smoke_col_1x1_debug_run1year_snw30percent_snwgrain compare ip180472b missing-data
MISS badger_intel_smoke_col_1x1_debug_run1year_snwITDrdg compare ip180472b missing-data
MISS badger_intel_restart_col_1x1_snwgrain_snwITDrdg compare ip180472b missing-data

FAIL badger_intel_restart_col_1x1_bgcISPOL compare ip180472b different-data
FAIL badger_intel_restart_col_1x1_zsal compare ip180472b different-data
FAIL badger_intel_restart_col_1x1_thermo1 compare ip180472b different-data
FAIL badger_intel_restart_col_1x1_alt04 compare ip180472b different-data

The 3 tests with missing data for the regression are the new snow models tests.
The 4 tests failing the regression all appear to be roundoff differences, as discussed in a comment above.

[proteanplanet]

I have carefully inspected the code, and could not find any issues. As part of this, I also carefully read the new documentation, which I found to be clear, concise and informative.