CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 131 forks source link

Initial, partial deprecation of zsalinity #413

Closed eclare108213 closed 1 year ago

eclare108213 commented 1 year ago

base_suite: all tests are BFB except those with solve_zsal = true, as expected.

162 of 167 tests PASSED 0 of 167 tests PENDING 0 of 167 tests MISSING data 5 of 167 tests FAILED

FAIL conda_macos_smoke_col_1x1_debug_run1year_zsal run FAIL conda_macos_smoke_col_1x1_debug_run1year_zsal test FAIL conda_macos_smoke_col_1x1_debug_run1year_zsal compare ibased33 different-data FAIL conda_macos_restart_col_1x1_zsal run FAIL conda_macos_restart_col_1x1_zsal test

The zsalinity tests have now been removed from the scripts.

eclare108213 commented 1 year ago

Because solve_zsal is so pervasive throughout the code, I decided not to wrap every occurrence with #ifdef UNDEPRECATE_ZSAL. The changes in this PR should be sufficient to alert the community that zsalinity is being deprecated, in this initial step (but CICE also needs to be checked to make sure it aborts as expected). Testing associated with the E3SM Icepack merge will ensure that complete zsalinity deprecation does not cause unanticipated problems.

eclare108213 commented 1 year ago

CICE aborts with -s zsal, as expected. Nothing more needs to be done there unless we want to remove zsal from the test suites now.

(icepack_init_parameters) zsalinity is being deprecated
    (icepack_warnings_setabort) T :file /Users/eclare/E3SMecosystem/CICE-Consortium/sandboxes/icepack.merge/cice.testing/icepack/columnphysics/icepack_parameters.F90 :line         1066
 (icepack_warnings_aborted) ... (icepack_init_parameters)
eclare108213 commented 1 year ago

I added the change to icepack_warnings here, for convenience. If we remove the zsal options here, then the code won't compile, and warning messages are only provided when the code aborts a run. So I think we should keep them for now (also in the spirit of being able to easily reinstate the code if needed).

apcraig commented 1 year ago

It's fine to leave them. But they are no longer in the test suite (zsal tests are removed), so deleting set_nml.zsal and set_env.zsal should have no impact. I agree we cannot remove zsal as a namelist setting in ice_in though, so given that, maybe it's fine to leave the set_nml and set_env files as we do need to do another cleanup of the zsal namelist later. Thanks.

eclare108213 commented 1 year ago

Thanks @njeffery. @apcraig that's right. I'd like to keep the zsal options available in case a user actually uses -s zsal, so that they'll get a warning message with the abort.

eclare108213 commented 1 year ago

I need to make one more correction to the docs

eclare108213 commented 1 year ago

Never mind - there's not actually anything related to zsalinity in the Icepack index. So this PR is good to go.

apcraig commented 1 year ago

Will merge when GHActions is done.