Deprecate zsalinity

[apcraig]

PR checklist

• [X] Short (1 sentence) summary of your PR: Deprecate zsalinity

• [X] Developer(s): apcraig

• [X] Suggest PR reviewers from list in the column to the right.

• [X] Please copy the PR test results link or provide a summary of testing completed below. Icepack test suite is bit-for-bit on cheyenne with intel, gnu, and pgi EXCEPT for pgi with bgc (this must be a compiler optimization issue with the code changes). https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#13e1c1dc418c7c38dedb1b9e76b6f72e53dd801a. Final CICE with zsalinity deprecation testing is underway, but expected to be bit-for-bit in full test suite on cheyenne with intel, gnu, pgi EXCEPT pgi with bgc on (passes but not bit-for-bit). Results for CICE for gnu and pgi are here, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#f836c7d5b39f7c3dc272ab50f6db615341110343

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

  • [X] bit for bit except PGI with bgc turned on
  • [ ] different at roundoff level
  • [ ] more substantial

• Does this PR create or have dependencies on CICE or any other models?

  • [ ] Yes
  • [X] No

• Does this PR add any new test cases?

  • [ ] Yes
  • [X] 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/.)

  • [X] Yes
  • [ ] No, does the documentation need to be updated at a later time?
    • [ ] Yes
    • [ ] No

• [X] Please provide any additional information or relevant details below:

• Closes #439

• Remove basic zsalinity code, but maintain Icepack interfaces for backwards compatibility. If solve_zsal is set to true, Icepack will abort.

• Includes removing "bgc_S" variables and implementation

• Update documentation

• Will PR CICE changes and update Icepack in CICE once this PR is merged.

• Also tested this version of Icepack with an unmodified version of CICE to confirm backwards compatibility. CICE will build and run bit-for-bit with this version of Icepack with deprecated zsalinity.

[eclare108213]

@njeffery - Can the brine height tracer be run without zsalinity, i.e. with mushy thermo? My plan has been to deprecate zsalinity but not hbrine, and I'm wondering whether that make sense. Our tests only have them both turned on, not hbrine by itself.

[njeffery]

Definitely. The brine height tracer is needed for the biogeochemistry and doesn't depend on the specific halodynamics.

[edited to remove email signature info]

[eclare108213]

Excellent. Correction: we do have tests with brine tracer on and zsalinity off in both icepack and cice. They are both on only for the zsal test.

[apcraig]

Good questions about the deprecation. I kept the zsalinity public interfaces and arguments for the time being for backwards compatibility. They are not needed for the Icepack driver or a future version of CICE, but other users might need them. If a user has an older version of CICE or uses Icepack in their ice model (MPASSI), then an upgrade to this version of Icepack should not break anything. They can compile and run (with zsalinity off) without having to change any of their code. If they turn on zsalinity, the code should abort. If we remove the public interfaces and arguments, then an Icepack upgrade could/will immediately fail to build and the user will have to modify calls into Icepack to get things working again, even if they are not using zsalinity.

So we can take either approach. Provide full backwards compatibility by maintaining zsalinity arguments/interfaces that have no impact on the code. Or remove that code and possibly force all users to update their "driver" when they bring in a new version of Icepack, with no real benefit. There are pros and cons to both approaches.

I tested the updated version of Icepack (the one in this PR) with the baseline version of CICE (with all the zsalinity code/calls) to make sure it truly is backwards compatible and bit-for-bit. Of coarse, CICE only works in that mode if zsalinity is off.

[apcraig]

If you feel we should fully deprecate the Icepack interfaces for zsalinity, I can certainly do that easily.

[apcraig]

I think hbrine is tested with bgc. Do we need to add a test to Icepack and/or CICE with just hbrine without bgc? Is that possible? If so, what other options should be turned on with it?

[apcraig]

Is there anything else to do with the PR, is it ready to merge? Please see also https://github.com/CICE-Consortium/CICE/pull/851, that also needs a review.

[apcraig]

I just rebased this, should be ready for review and merge.

[apcraig]

I'm pretty sure the brine tracer is preserved with these changes. The full test suite runs and is basically bit-for-bit. Maybe we need to add a new brine tracer test at some point.