Add Tf to icepack interface calls and constant tfrz_option.

[dabail10]

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

• Short (1 sentence) summary of your PR:
  Add Tf to icepack interface calls and constant tfrz_option.
• Developer(s):
  dabail10 (D. Bailey), apcraig
• 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.
  Latest implementation is bit-for-bit on cheyenne with 3 compilers, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#218c05125c9632886db26abeb0d4ec7ded72a6ef with the following caveats
  • the log files are bit-for-bit, the restart files change values for Tf on non-ice gridcells
  • dynpicard still fails (need to merge in latest changes from Consortium main)
  • the 10 year test runs, smoke_gx1_144x2_gx1prod_long_run10year, diverge after 3 years
• 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 Icepack or any other models?
  • Yes
  • No
• Does this PR update the Icepack submodule? If so, the Icepack submodule must point to a hash on Icepack's main branch.
  • 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/. A test build of the technical docs will be performed as part of the PR testing.)
  • Yes
  • No, does the documentation need to be updated at a later time?
    • Yes
    • No
• Please provide any additional information or relevant details below:
  This changes answers in that all of the temperatures at open ocean points change for tfrz_option not equal to 'minus1p8' in the history and restart files. The run log files are bfb.

[eclare108213]

I just have some picky comments. Otherwise this looks good.

[eclare108213]

Suggestions for the diagnostic output

• write out what the value of Tocnfrz is, here
• switch the order of the logic: if ... 'constant' then elseif ... 'minus1p8' and add a comment that this base is for backwards compatibility and will be deprecated

[apcraig]

I confirmed the Icepack hash independently and am running a small-ish test suite now, but this looks fine. Will merge once minor changes are made and @eclare108213 approves.

[apcraig]

I ran Icepack before and after the Tf addition, that's entirely bit-for-bit for the full icepack test suites on cheyenne with 3 compilers.

I ran the quick_suite and base_suite on cheyenne+gnu, and I do not get bit-for-bit for all cases. The following cases are NOT bit-for-bit,

FAIL cheyenne_gnu_restart_gx3_4x4_alt04 complog cice.968948928a.221024-173018 different-data
FAIL cheyenne_gnu_smoke_gx3_4x4_alt04_debug_short complog cice.968948928a.221024-173018 different-data
FAIL cheyenne_gnu_restart_gbox128_4x2_boxnodyn_short complog cice.968948928a.221024-173018 different-data
FAIL cheyenne_gnu_restart_gbox128_4x2_boxnodyn_debug_short complog cice.968948928a.221024-173018 different-data
FAIL cheyenne_gnu_restart_gbox128_2x2_boxadv_short complog cice.968948928a.221024-173018 different-data
FAIL cheyenne_gnu_restart_gx1_4x2_bgcsklclim_medium complog cice.968948928a.221024-173018 different-data

This has to do with the changes in E3SM-Project/Icepack#10 and not these mods.

[dabail10]

Ok. I fixed the indents and messages. Also, I added Tf to the icepack_aggregate calls in the drivers and unittests.

[apcraig]

I'm wondering if we should add the hin_min namelist and get things bit-for-bit for all cases now. This is unrelated to the Tf changes, but does need to be done as part of the update of Icepack. That could be part of this PR or a separate one. Maybe I'll add that change and push to this branch before we merge, is that OK @dabail10?

[dabail10]

That sounds fine.

[apcraig]

Checking the CICE results carefully. There are some cases that are not bit-for-bit due to changes in the Icepack Tf implementation. Have figured out why, need to run a few more tests. More soon.

[apcraig]

Just FYI, I rebased this branch and force pushed, so history changed, but just on this branch.

[apcraig]

The last thing to do is add the updated Icepack, waiting on E3SM-Project/Icepack#14 to be merged. Then I'll update Icepack here and retest everything.

[eclare108213]

There is a mix of values here for hi_min, sometimes 0.1, sometimes 0.01. Is that intentional?

[apcraig]

There is a mix of values here for hi_min, sometimes 0.1, sometimes 0.01. Is that intentional?

Yes, that is consistent with the values that were hardcoded before for certain configurations. Now we have to set it in the namelist. It's possible those are not ideal or best values, but this is just testing. The default is basically 0.01 and a few cases use 0.1, as before.

[eclare108213]

Okay, just checking!

[apcraig]

The latest implementation is bit-for-bit on cheyenne with 3 compilers, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#218c05125c9632886db26abeb0d4ec7ded72a6ef with the following caveats

• the log files are bit-for-bit, the restart files change values for Tf on non-ice gridcells
• dynpicard still fails (need to merge in latest changes from Consortium main), not related to this PR.
• the 10 year test runs, smoke_gx1_144x2_gx1prod_long_run10year, diverge after 3 years

This is still bit-for-bit with the Consortium main #422117f from Oct 6, see #102.

It's rather curious that only some of the test cases require the _old tfrz_option setting while others do not. Also, the 10 year run does diverge after 3 years. So these changes and their fixes to recover bit-for-bit must not be perfect. There still is likely a change in the computation somewhere. The _old tfrz_options should be removed at the appropriate time.

This is ready for review/merge. I'll wait until the end of the day to merge in case there are any additional comments/concerns.

[dabail10]

I'm not sure what to suggest. Perhaps an ice free cell is being set to the new freezing point and then it grows ice? I don't know why it would take three years though. Regardless, this is the right thing to do.