Update icepack_step_therm2, make fsd arguments optional

[apcraig]

PR checklist

• Short (1 sentence) summary of your PR:
  Update icepack_step_therm2, make fsd arguments optional
• Developer(s):
  apcraig
• 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.
  Full test suite run on cheyenne for Icepack+intel and CICE+gnu, all tests pass and bit-for-bit.
• 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
    • No
• Please provide any additional information or relevant details below:

Also update interface documentation.

[eclare108213]

I'm still confused about how the optional arguments work. Do the FSD-related arguments need to be optional in lateral_melt? E.g. fiso_ocn is optional in step_therm2 but not in the routines called from there, like lateral_melt and add_new_ice. Why would the FSD args be different?

[apcraig]

@eclare108213, that's a good question. The optional argument attribute is not required in lower level routines. You may not be able to check whether that argument is present anymore in the lower level routines, but as long as you DON'T use an optional argument that was not passed above, the code should compile and run fine whether the optional attribute is on the lower level routines or not. As we know, if you don't pass an optional argument then try to use it, whether it's declared optional or not in the lower level routines, you should get a runtime error. The lower level optional arguments don't change that. If you prefer, I can remove the optional arguments on the lower level routines for consistency. I don't feel strongly that it has to be one way or the other, but we can define a standard if you like. There may be times where that optional argument attribute has to be set in lower level routines, but hopefully that's not the case most of the time.

[eclare108213]

I'd prefer that we do it consistently -- otherwise it's too confusing when implementing new arguments (or fixing old code). There was an issue a few months back, maybe with a snow variable? and now I worry that the code isn't as robust with all the optional args. Does the optargs unit test provide a template for how this is supposed to work? If so, maybe that could be pointed out in the docs, or some pseudo-code provided there. I searched for 'optional' and didn't find anything like that.

[apcraig]

I updated the lower level optional arguments and retested everything, still working fine.

Section 5.2 of the Icepack users guide talks about optional arguments. It's relatively current, but let me update that a bit.

[apcraig]

https://cice-consortium-icepack--440.org.readthedocs.build/en/440/developer_guide/dg_col_phys.html

[lettie-roach]

This looks fine to me. Did your test to check BFB-ness have tr_fsd=True?

[apcraig]

I ran a full test suite, that includes tests with tr_fsd=true, all results are bit-for-bit.