zsalinity test #344
zsalinity test #344
Conversation
|
Hi Elizabeth,
I would try running with z_tracers = true. This shouldn't be needed, but it's possible that some of the initialization/activation for zsalinity is tied to this flag. If all is working well with zsalinity, then it shouldn't make a difference.
I'd stick with the default forcing.
n
…________________________________
From: Elizabeth Hunke <notifications@github.com>
Sent: Saturday, December 12, 2020 1:34:28 PM
To: CICE-Consortium/Icepack
Cc: Jeffery, Nicole; Mention
Subject: [EXTERNAL] [CICE-Consortium/Icepack] zsalinity test (#344)
* Short (1 sentence) summary of your PR:
Adding smoke and restart tests for zsalinity in Icepack
* Developer(s):
@eclare108213<https://github.com/eclare108213> for this test, @njeffery<https://github.com/njeffery> for the original zsalinity code
* 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.
https://github.com/CICE-Consortium/Test-Results/wiki/3ca0dda63c.badger.intel.20-12-12.193908.0
* 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 CICE or any other models?
* 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/.)
* Yes
* No, does the documentation need to be updated at a later time?
* Yes? There's a new -s zsal option
* No
* Please provide any additional information or relevant details below:
This PR only adds and option and tests, with no changes to the original code.
@njeffery<https://github.com/njeffery>, please take a look at the output linked here for the smoke test, expecially the configuration. The brine tracer seems to be working and the average salinity is changing, so the zsalinity code appears to be active. I'm using the default forcing, and maybe this test would be better with one of the BGC forcing data sets (ISPOL or NICE)? I have not yet been able to get a zsalinity case to run in CICE.
logs.tar.zip<https://github.com/CICE-Consortium/Icepack/files/5683716/logs.tar.zip>
@apcraig<https://github.com/apcraig> @dabail10<https://github.com/dabail10> Is the documentation for -s options entirely automated, or do I need to add something to readthedocs?
________________________________
You can view, comment on, or merge this pull request online at:
#344
Commit Summary
* zsalinity test
File Changes
* A configuration/scripts/options/set_env.zsal<https://github.com/CICE-Consortium/Icepack/pull/344/files#diff-1b60c73e0716d6a118c2f50c3d648da071dbc43587df0267499c964b2a5fafab> (10)
* A configuration/scripts/options/set_nml.zsal<https://github.com/CICE-Consortium/Icepack/pull/344/files#diff-1f1c7c2c9c5701c2ef7f33659bb8f6f9eae0ace0d37b31c04bbaf729fe710f90> (5)
* M configuration/scripts/tests/base_suite.ts<https://github.com/CICE-Consortium/Icepack/pull/344/files#diff-6d82644a558715d6c241d86a51d3865bf87c09b0bdc085326cdda51ef40be7d9> (2)
Patch Links:
* https://github.com/CICE-Consortium/Icepack/pull/344.patch
* https://github.com/CICE-Consortium/Icepack/pull/344.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#344>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB6UV2RLEYX435VJGJPBEIDSUPHVJANCNFSM4UYTLIVQ>.
|
|
For readthedocs to work, an update from master or an update to doc/requirements.txt will be needed. |
|
I'm not convinced that this test (or the zsalinity code) is working correctly. In Icepack (this PR), the salinity changes slightly during a run but seems relatively impervious to major changes like different initial conditions, forcing, etc. Its values are always like this: avg salinity (ppt) = 2.30068935292552013 In CICE (a draft PR I'll submit soon), the values look much more reasonable, although high. (@njeffery turning z_tracers on did allow CICE to run, thanks.) Since this needs to be understood, I don't think we should merge the PR as it is, but there are options:
I'm in favor of (1) but fine with closing this PR for now and going with (2). |
|
I am fine with any of the 3 options. I don't think it's a problem to add the test even if the answers change later if it's useful. We are often changing results for a handful of tests at a time because of a change in implementation or otherwise. |
|
I fixed a bug in the initialization in which the solve_zsal parameter wasn't set after being read in from namelist, so it was reset to false when queried. The diagnostics also weren't being printed correctly. Now the zsalinity output from Icepack behaves similarly to CICE. Note that z_tracers=T is not needed for Icepack, but is for CICE. I'm comfortable with merging the zsalinity tests for both Icepack and CICE, understanding that there still may be bugs. It's better to test... |
Adding smoke and restart tests for zsalinity in Icepack
@eclare108213 for this test, @njeffery for the original zsalinity code
https://github.com/CICE-Consortium/Test-Results/wiki/3ca0dda63c.badger.intel.20-12-12.193908.0
This PR only adds and option and tests, with no changes to the original code.
@njeffery, please take a look at the output linked here for the smoke test, expecially the configuration. The brine tracer seems to be working and the average salinity is changing, so the zsalinity code appears to be active. I'm using the default forcing, and maybe this test would be better with one of the BGC forcing data sets (ISPOL or NICE)? I have not yet been able to get a zsalinity case to run in CICE.
logs.tar.zip
@apcraig @dabail10 Is the documentation for -s options entirely automated, or do I need to add something to readthedocs?