-
Notifications
You must be signed in to change notification settings - Fork 132
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 in the CICE driver #732
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Failed tests on badger These always fail because 40 pes are not available: These are expected to fail with ktherm=0 in the baseline, 1 in the new test: This one is failing in EAP:
These tests also failed in the baseline case (segfault) as well as this test suite, and so need to be debugged but are probably independent of the changes in this PR: 54 tests are showing missing data. The baseline suite shows the runs as still pending even though they are not in the queue. Badger seems very flaky. @dabail10 @apcraig could one of you launch a CICE test from this branch, please? Edit for clarification: please run a base_suite with regression I'll look into the EAP out-of-bounds error and the bgc segfaults. |
I am running the full test suite on the ponds on cheyenne. I will try to get to this one on Monday. Did you also try running with setting -DUNDEPRECATE_0layer? |
I have not yet tried -DUNDEPRECATE_0layer - will try that on a single run now. |
Hi Dave,
Were you able to test the 0-layer thermo changes? I did try -DUNDEPRECATE_0layer and uncovered a problem, which I think I fixed on Friday (so update your download).
I've gotten the krdg_partic and krdg_redist changes almost done, using the same branch, and I'd like to know whether the tests are coming out okay before I commit anything. Easier to revert and clean up before anything is committed! I'll also try testing here again, but it's just really flaky - very frustrating.
|
I will test it out now. I ran the cesmponds over the weekend. |
A lot of FAILS here, but most are non-bfb and are consistent with the change from ktherm=0 to 1 in alt01, boxadv, boxnodyn, and boxrestore. Also, the dynpicard have different answers. The only ones that are not different data is: FAIL cheyenne_intel_smoke_gbox128_2x2_boxadv_debug_short run -1 -1 -1 Here is the error in one: At line 1827 of file /glade/work/dbailey/CICE_elizabeth/cicecore/cicedynB/dynamics/ice_dyn_eap.F90 |
Thank you. I will look into the EAP array error. |
FAILed test results with both sets of deprecations, on badger: These always fail because 40 pes are not available on badger: These are expected to fail with ktherm=0 in the baseline, 1 in the new test: These are expected to fail with krdg_partic=0 and krdg_redist=0 in the baseline, defaults to 1 in the new test: These are expected to fail due to the namelist changes for both sets of deprecations. The boxadv runs are now completing but with different data, as expected: These tests also failed in the baseline case (segfault) as well as this test suite, and so are probably independent of the changes in this PR: I think these results are acceptable, although there are a lot of missing regression tests so I'm not 100% confident that everything is BFB. I also tested that the code compiles with the UNDEPRECATE flags set. Results will be different due to the namelist changes (not tested). |
…ons" This reverts commit 95c289a.
New test results from badger with just the 0-layer deprecation: 375 measured results of 375 total results Same failures as above, except now the alt02_debug_short test is missing and the other alt02 test (which failed above) now passes. Cheyenne tests are underway and more reliable. |
Test results from cheyenne look fine, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#11136cb0cf7a875efd1c08019b014bbc2a96ec4d. Looks like alt01, boxnodyn, boxadv, and boxrestore changed answers. |
) * initial 0-layer thermo deprecation * capitalization matters for cpps * set_nml.boxadv needs thermo turned on * deprecate old ridging participation and redistribution functions * Revert "deprecate old ridging participation and redistribution functions" This reverts commit 95c289a.
In this initial deprecation step, the zero-layer (ktherm=0) thermodynamics option is removed via cpp flags so that it can easily be reinstated if necessary. Documentation is also updated.
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 and the cpp flags in both CICE and Icepack. 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!)
@eclare108213
See https://github.com/CICE-Consortium/Test-Results/wiki/7b5538dd9a.badger.intel.22-07-28.174752.0.
373 measured results of 373 total results
293 of 373 tests PASSED
0 of 373 tests PENDING
54 of 373 tests MISSING data
26 of 373 tests FAILED
See comments below for explanation.
This PR uses the UNDEPRECATE_0LAYER cpp flag to remove the 0-layer thermodynamics (ktherm=0) option, as in the associated Icepack PR #394. It also removes the internally defined heat_capacity option, which is always false when ktherm=0 and true for the other thermodynamic options. The only namelist change for the 0-layer change is to ktherm. Various set_nml option files that had ktherm=0 were changed to ktherm=1 (BL99). The cice4 restart conversion fortran code is also modified slightly.
To undeprecate the 0-layer option, set -DUNDEPRECATE_0LAYER in cice.settings' CPPDEFS and adjust namelist values as needed.
This PR 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 in Icepack#394.