zsal option for testing zsalinity code

[eclare108213]

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

• Short (1 sentence) summary of your PR:
  Provides -s zsal option for testing zsalinity code and improving code coverage
• 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.

https://github.com/CICE-Consortium/Test-Results/wiki/9d9b65ce8f.badger.intel.20-12-18.233253.0

193 measured results of 193 total results
189 of 193 tests PASSED
0 of 193 tests PENDING
0 of 193 tests MISSING data
4 of 193 tests FAILED

The 4 failures are the usual badger 40-proc fails. The smoke test in this batch of test results is for a 1-year run, which took so long that I changed it to a 5-day test with debugging turned on in the base_suite modification.

• 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 Icepack 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/. A test build of the technical docs will be performed as part of the PR testing.)
  • Yes
  • No, does the documentation need to be updated at a later time?
    • Yes
    • No
• Please provide any additional information or relevant details below:

I'm not sure whether documentation must be manually updated when new tests are added.

See CICE-Consortium/Icepack#344 includes roughly equivalent tests for Icepack.
Note that z_tracers=T is not needed for Icepack, but is for CICE.

[eclare108213]

plots.tar.gz

Here are annual timeseries plots and cice.runlog files for the zsal run with the new option and for the default (mushy). The initial conditions are different but overall the behavior is similar and more or less as expected. I'm not sure why the zsal salinity starts so high; it decreases through the year and does not reach the low values seen in mushy.

@njeffery if you would please take a look at cice.runlog.zsal in the attached file, where the salinity profile is printed out through the run. It looks okay to me but you're more of an expert on this. If you think it's okay, I'll add this as part of our regular test suite.

[njeffery]

It looks like it's working this time. That's great. It's not a confirmation that everything's working precisely as it should, but I'm comfortable with including as a testsuite.

…

________________________________ From: Elizabeth Hunke <notifications@github.com> Sent: Friday, December 18, 2020 4:22:16 PM To: CICE-Consortium/CICE Cc: Jeffery, Nicole; Mention Subject: [EXTERNAL] Re: [CICE-Consortium/CICE] zsal option for testing zsalinity code (#548) plots.tar.gz<https://github.com/CICE-Consortium/CICE/files/5718484/plots.tar.gz> Here are annual timeseries plots and cice.runlog files for the zsal run with the new option and for the default (mushy). The initial conditions are different but overall the behavior is similar and more or less as expected. I'm not sure why the zsal salinity starts so high; it decreases through the year and does not reach the low values seen in mushy. @njeffery<https://github.com/njeffery> if you would please take a look at cice.runlog.zsal in the attached file, where the salinity profile is printed out through the run. It looks okay to me but you're more of an expert on this. If you think it's okay, I'll add this as part of our regular test suite. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#548 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB6UV2TOC6RJ5GRIT5I5BLLSVPP2RANCNFSM4VBYFYEQ>.

[njeffery]

The initial run looks reasonable. There still may be issues with the code, but at least the zsalinity portion is running.

[apcraig]

I think the restart test should be done with threading (say 8x2). Does the smoke test with debug for 5 days run so slowly that it needs medium?

[eclare108213]

I'll try these tests on badger and report back.

[eclare108213]

My testing ran into problems where, for history fields defined outside of ice_history.F90 (mechred, ponds, bgc), the code was trying to accumulate arrays that had not been allocated because there were no similarly shaped fields turned on in the main history namelist. It shouldn't have been trying to accumulate them in the first place -- I think I fixed this. Since the changes also affect mechanical redistribution and melt pond output, I'm running a full base_suite again. Note: base_suite might not fully test the history functionality, something we should probably look into as part of our code coverage work. A particular case:

histfreq = 'd'
f_aice = 'd' and no other history variables = 'd'
f_fbri = 'm' caused the brine height tracer to be accumulated but aD3C was not allocated since aice is 2D.

Test results. The fails are the usual 40-proc tests.

I'm comfortable with this PR and the related Icepack PR being merged now.

[apcraig]

Do the history fixes relate at all to #521?

[eclare108213]

Yes, #521 is definitely related. That is the same error I was getting, but for the a3Dc array instead of a2D. My changes might have fixed that too, but additional conditionals might still be needed in ice_history.F90. I only fixed the ones in the other ice_history_*.F90 modules.

[apcraig]

@eclare108213, Testing on other compilers picked up a problem with the debug zsal test, https://github.com/CICE-Consortium/Test-Results/wiki/f5f487f973.cheyenne.gnu.20-12-23.235355.0. The test fails with the gnu compiler at line 1013 in icepack/columnphysics/icepack_algae.F90. Some quick debugging indicates bphin_N is not initialized because nbtrcr is zero. This only fails with the gnu compiler, I assume the gnu compiler with debug flags trap uninitialized values while the other compilers might only do so if they are initialized to invalid values.

I don't have any further information, but could debug further if you like. I guess this is an interaction of the bio and algae settings? Maybe algae shouldn't be called at all unless bgc is on or maybe there is some other logic that belongs here? Let me know if you want me to create a separate issue.

[eclare108213]

Algae shouldn't be called unless the bgc is turned on, either skeletal or zbgc. Looks like it's getting called because z_tracers=T, which @njeffery suggested to turn on in order to initialize zsalinity. So we either need to increase nbtrcr or else debug the initialization of zsal...

Edit: increasing nbtrcr would be a reasonable kludge to avoid the crash, but the correct solution is to turn off z_tracers and fix the initialization. I'll try to look at this.

I noticed that 'zbio' is used in two different ways in the code, as a keyword and as a subroutine name. That's probably not the problem in this case but still ought to be fixed.