Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate 0-layer thermodynamics #394

Merged
merged 7 commits into from
Jul 31, 2022

Conversation

eclare108213
Copy link
Contributor

@eclare108213 eclare108213 commented Jul 19, 2022

In this initial deprecation step, code is removed via cpp flags so that it can easily be reinstated if necessary. Documentation is more difficult to comment out but changes are made there also.

The intent is for this cpp'd code to be released, and then for these changes to be made permanent in a followup release, removing all of the cpp'd code, the cpp flags, and the entire icepack_therm_0layer.F90 module. The initial code release allows for feedback from the user community earlier during the process, before the proposed changes become more difficult to reinstate. (So speak up!)

  • Short (1 sentence) summary of your PR:
    Initial deprecation of the 0-layer thermodynamics option and related documentation
  • Developer(s):
    @eclare108213
  • 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.

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

Failures are due to changed namelist values, as expected:
FAIL badger_intel_smoke_col_1x1_alt01_debug_run1year compare depr_baseline different-data
FAIL badger_intel_smoke_col_1x1_alt02_debug_run1year compare depr_baseline different-data
FAIL badger_intel_restart_col_1x1_alt01 compare depr_baseline different-data
FAIL badger_intel_restart_col_1x1_alt02 compare depr_baseline different-data

  • How much do the PR code changes differ from the unmodified code?
    • bit for bit except for changed alt01 and alt02 option files
    • different at roundoff level
    • more substantial
  • Does this PR create or have dependencies on CICE or any other models?
    • Yes CICE will need to be altered also
    • No
  • Does this PR add any new test cases?
    • Yes
    • No but I am planning to change those with ktherm=0 to ktherm=1
  • 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:

This PR uses the UNDEPRECATE_0LAYER cpp flag to remove the 0-layer thermodynamics (ktherm=0) from icepack_therm_0layer.F90 and calls to it. It also removes the internally defined heat_capacity option, which is always false when ktherm=0 and true for the other thermodynamic options. The parameter kseaice is specific to the 0-layer thermodynamics and therefore also removed. The only namelist change for the 0-layer thermo deprecation is to ktherm. To undeprecate the 0-layer option, set -DUNDEPRECATE_0LAYER in icepack.settings' CPPDEFS and adjust namelist values as needed.

Also updates a machine file for badger.

Code and documentation related to the old ridging participation and redistribution functions was also deprecated, and then reverted (not deprecated). See comments below.

Copy link
Contributor

@dabail10 dabail10 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 great! A lot of work. Because it is a draft PR, we do not see the readthedocs to be sure the comments are working.

@eclare108213
Copy link
Contributor Author

Thanks. I'm struggling to get the testing set up and working. The baseline runs don't complete on my laptop, and I'm not sure if the problem is the code or the laptop -- no info in the output. Could you pull down this branch and try it, quickly? I'll try badger.

@eclare108213
Copy link
Contributor Author

readthedocs output is here
https://cice-consortium-icepack--394.org.readthedocs.build/en/394/
(click on Details above?)

@dabail10
Copy link
Contributor

Ok. Just set off the base_suite on cheyenne.

@eclare108213
Copy link
Contributor Author

@dabail10 ran the icepack base suite on cheyenne and got the following:

165 measured results of 165 total results
160 of 165 tests PASSED
0 of 165 tests PENDING
1 of 165 tests MISSING data
4 of 165 tests FAILED

The fails are because the ktherm value changes in alt01 and alt02:

FAIL cheyenne_intel_smoke_col_1x1_alt01_debug_run1year compare icepack.76ecd418d2.220716-085004 different-data
FAIL cheyenne_intel_smoke_col_1x1_alt02_debug_run1year compare icepack.76ecd418d2.220716-085004 different-data
FAIL cheyenne_intel_restart_col_1x1_alt01 compare icepack.76ecd418d2.220716-085004 different-data
FAIL cheyenne_intel_restart_col_1x1_alt02 compare icepack.76ecd418d2.220716-085004 different-data

The missing is the snwITDrdg/snwgrain test. Why is this not in the standard tests?

@eclare108213 eclare108213 changed the title Deprecate 0-layer thermodynamics Deprecate 0-layer thermodynamics and old ridging functions Jul 28, 2022
@eclare108213 eclare108213 marked this pull request as ready for review July 28, 2022 21:19
@kshedstrom
Copy link

I'd like to note that when using the ridging scheme from within SIS2, I have found the new ridging options to be unstable while the old ones are not. For example:

WARNING from PE     4: Negative ice volume after ridging:    181    13    9.2766E-57 -7.3863E-67
FATAL from PE     4: Input to adjust_ice_categories, non-zero snow mass rests atop no ice.

This is a month and a half into a regional run - I haven't thoroughly investigated this, but I could.

@eclare108213
Copy link
Contributor Author

@kshedstrom Thanks for the heads-up. I wasn't aware of any issues like this, and I'm interested to know what the situation is in your model configuration that instigates it. We are planning to do some further development on the ridging scheme, and so keeping the old ridging functions as a backup/alternative for comparisons seems wise. I'll undeprecate the krdg options before we merge this PR. Thank you!

@eclare108213 eclare108213 changed the title Deprecate 0-layer thermodynamics and old ridging functions Deprecate 0-layer thermodynamics Jul 29, 2022
@kshedstrom
Copy link

Thank you!

The case has aicen going into the ridging with: (5.1e-10, 3.1e-19, 0, 9.27e-57, 0)
vicen = (2.9e-17, 7.2e-29, 0, 9.3e-67, 0)

After ridging: aicen = (5.1e-10, 3.1e-19, 0, 9.27e-57, 4.16e-66)
vicen = (2.9e-17, 8.0e-26, 0, -7.4e-67, 2.9e-66)

Clearly, these are tiny scraps of ice which don't necessarily need to ridge. In my SIS2 code, the negative number gets set to zero, but then it can't support the tiniest scrap of snow - something we can disappear as well.

@apcraig apcraig merged commit 3cb1746 into CICE-Consortium:main Jul 31, 2022
@eclare108213
Copy link
Contributor Author

Ah, the residual ice issue. See CICE-Consortium/CICE#645. Please, if you have a chance, put your configuration in a comment in that issue. I'd really like to know why the ocean isn't melting these very tiny scraps of ice! Maybe we should force it to do so, i.e. just zap the ice that is smaller than the dynamics and/or thermodynamics minimum concentration and/or volume (there are limits for numerical stability), and track it for conservation. Thanks!

eclare108213 added a commit to E3SM-Project/Icepack that referenced this pull request Aug 1, 2022
* Deprecate CESM pond option (CICE-Consortium#393)

* Deprecate CESM pond option

* Update comments in documentation

* Deprecate 0-layer thermodynamics (CICE-Consortium#394)


* 0-layer deprecation - initial

* update doc

* update test options and abort if ktherm=0

* deprecate old ridging participation and redistribution functions

* Revert "deprecate old ridging participation and redistribution functions"

This reverts commit 8aa2f36.

Co-authored-by: Tony Craig <apcraig@users.noreply.github.com>

Co-authored-by: David A. Bailey <dbailey@ucar.edu>
Co-authored-by: Elizabeth Hunke <eclare@lanl.gov>
@kshedstrom
Copy link

Note that my troubles are in MOM6-SIS2, just calling Icepack for the ice ridging. I would have to ask @GFDL-Hallberg about conservatively dealing with this.

@kshedstrom
Copy link

I mean @Hallberg-NOAA

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.

4 participants