Update snwgrain implementation to fix bug in snow sublimation

[apcraig]

PR checklist

• Short (1 sentence) summary of your PR:
  Fix bug in snwgrain snow sublimation

• Developer(s):
  apcraig, njeffery

• 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.
  All test pass and bit-for-bit in Icepack and CICE except snwgrain tests, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#28fdbebf4a3b98485c1e977344fd402f29f6a0b6, https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#23a8faf882010e7640232ac8216f2416490aa296

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

  • bit for bit except snwgrain
  • 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
    • No

• Please provide any additional information or relevant details below:

• fix bug in sublimation of snow

• check allocation of snow aging table

needed for E3SM, reported by NJ.

[apcraig]

I implemented the allocation checks as deallocates then reallocate. I'm not sure what is going on there. The arrays should never be allocated ahead of time if snw_aging_table='snicar' or 'test'. If snw_aging_table='file', then the user defines those arrays. So, in the former case, the arrays should not be pre allocated and then icepack will allocate and fill them. In the latter, the arrays are allocated and filled by the driver/user and there is no allocation conflict. Maybe we should abort if snw_aging_table='snicar' or 'test' and the arrays are allocated?? Looking for some additional guidance here about why this is happening in E3SM.

[njeffery]

@apcraig : In E3SM, the snow tables read from streams are passed to icepack in icepack_init_parameters and allocated here first (snowage_tau, snowage_kappa, snowage_drdt0). Then init_icepack_snow_tracers is called where snowage_rhos , snowage_Tgrd and snowage_T are allocated and defined. The first three should be queried if (.not.allocated) allocate, but I think you're right about the last three. They should be fine as is.

[eclare108213]

I think the snow tables are now available internally in Icepack, so MPAS-SI doesn't need to read them from streams.

[njeffery]

It's not as flexible. Does Icepack overwrite the passed fields?

[eclare108213]

Icepack and CICE have a namelist flag to choose what method to use, snw_aging_table. Icepack itself doesn't read the data (and so snw_aging_table='snicar' is not available in Icepack). CICE uses that option, and so MPAS-SI should be able to also. Maybe the conflict here is only that CICE and MPAS-SI allocate fields in different places, at different times.

@apcraig, looking at the just the documentation, I don't understand the difference between the 'file' and 'snicar' options in CICE (and I don't remember how this worked).
https://cice-consortium-cice.readthedocs.io/en/main/user_guide/ug_case_settings.html#snow-nml
https://cice-consortium-icepack.readthedocs.io/en/main/user_guide/ug_case_settings.html#snow-nml

[apcraig]

First of all, the documentation on readthedocs is for the current Consortium main. The E3SM Icepack is different. So don't look at the readthedocs for this at the moment.

I had a closer look at the implementation and now understand what is happening. I will update the allocate/deallocate thing. I think Nicole had it right/closer than I did.

This is how it's implemented on the E3SM Icepack. snw_aging_table namelist has 3 possible values, 'snicar', 'test', and 'file'. In Icepack standalone, only 'test' is supported. This fills the 6 snowage arrays with test data internally. When 'file' is set, all 6 snowage fields have to be passed into icepack_parameters. 'snicar' is the weird one. With 'snicar', Icepack expects snowage_tau, kappa, and drdt0 to be passed in, but they are expected to be the "known" snicar ones. It then sets the rhos, Tgrd, and T. I think this is because the snicar dataset doesn't define the latter 3 fields in the file, they are just "known". And so we let Icepack define those. It would be better if there were a snicar input file that included the values of rhos, Tgrd, T and then the user would specify 'file'. So the problem really is the snicar setting. And that also comes down to whether you call icepack_init_snow before or after icepack_init_parameters with the snowage data passed in. Again, all this was done based on the snicar file that we had access to.

So let me refactor that a bit more, I now understand why E3SM is having problems with that allocate, it must call icepack_init_parameters before icepack_init_snow. I'll have an update soon.

[apcraig]

I updated the allocation checks, think I have it OK now. I pushed the changes and am rerunning the test suites, don't expect any problems with testing but will report them when done.

[apcraig]

Full test suites with Icepack and CICE all still good on cheyenne with latest update. This is ready for review and merge.

[njeffery]

Approved.

Tested in the step_snow integration (https://github.com/E3SM-Project/E3SM/tree/njeffery/seaice/icepack-step-snow) version of E3SM. Single year, 1D stand alone MPAS-SI runs with colpkg vs icepack with #428 are still BFB.

[apcraig]

Thanks for testing @njeffery.