Fix sfc_flux argument order in icepack_therm_vertical.F90

[kieranricardo]

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:
  Fixes set_sfcflux argument order in icepack_therm_vertical.F90, this caused errors for models using calc_tsfc=false. Addresses set_sfcflux arguments are switched in icepack_therm_vertical.F90 #512
• Developer(s):
  Kieran Ricardo
• 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.
  Passed GitHub actions tests: https://github.com/ACCESS-NRI/Icepack/actions/runs/13231902867/job/36930302099?pr=6
• How much do the PR code changes differ from the unmodified code?
  • bit for bit (except for models using calc_tsfc=false)
  • 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 document the changes in detail, including why the changes are made. This will become part of the PR commit log.

The call to set_sfcflux in icepack_therm_vertical.F90 has the fsurfn_f and fcondtopn_f arguments switched, causing issues for models using calc_Tsfc = .false.. This PR fixes these.

[anton-seaice]

This presumably will impact the CICE test that sets calc_Tsfc = .false.

If it makes sense to run CICE or Icepack tests, I can do them if that is useful @apcraig

[dabail10]

We should definitely run the Icepack / CICE tests here to make sure it does not affect configurations with calc_Tsfc = .true. Also, if it is answer changing for calc_Tsfc = .false. then we should do a QC test in CICE.

https://cice-consortium-cice.readthedocs.io/en/main/user_guide/ug_testing.html#quadratic-skill-validation-test

[eclare108213]

Good catch, thanks.

[dabail10]

I still think it would be useful to see the QC tests here.

[apcraig]

I fired off some test suites this morning, waiting to see what they report. We have no tests for Icepack with calc_tsfc=.false. so just checking whether all tests are bit-for-bit. We do have a test in CICE with calc_tsfc=.false. so checking results for CICE too.

My understanding is that this is a bug. calc_tsfc=.true. should be bit-for-bit. calc_tsfc=.false. should change some results, but in stand-alone mode, is it just a diagnostic bug? If true, what would QC tell us? Happy to do QC if it makes sense. Are you suggesting QC if answers are not bit-for-bit with calc_tsfc=.true.?

[dabail10]

I fired off some test suites this morning, waiting to see what they report. We have no tests for Icepack with calc_tsfc=.false. so just checking whether all tests are bit-for-bit. We do have a test in CICE with calc_tsfc=.false. so checking results for CICE too.

My understanding is that this is a bug. calc_tsfc=.true. should be bit-for-bit. calc_tsfc=.false. should change some results, but in stand-alone mode, is it just a diagnostic bug? If true, what would QC tell us? Happy to do QC if it makes sense. Are you suggesting QC if answers are not bit-for-bit with calc_tsfc=.true.?

I just think it would be good to evaluate the change with calc_tsfc = .false. If it is answer changing with calc_Tsfc = .true. then that would be an issue.

[eclare108213]

I'm not sure that our tests are able to evaluate whether calc_Tsfc=.false. would be climate-changing, because I don't think our forcing has the fields necessary to produce results equivalent to calc_Tsfc=.true.

[dabail10]

@kieranricardo did you evaluate this bug fix in your coupled system?

[anton-seaice]

@kieranricardo did you evaluate this bug fix in your coupled system?

The bug fixed version is the one we've been using, kieran might have records of what impact this had, but as its a bug we might not have paid too much attention to the old results

[apcraig]

All the standalone Icepack and CICE tests are bit-for-bit based on log and restart files. It could be that some diagnostic fields on history files are not bit-for-bit, we don't check that. Are there some history fields that I should turn on and compare with/without this change? Or should we just do the merge?

[anton-seaice]

I am surprised about set_nml.alt03 ? either way, i think it's good to just merge

[apcraig]

Yes, the alt03 results were bit-for-bit. Again, I don't think calc_tsfc affects the stand-alone model, although it might impact some diagnostic fields in the stand-alone model that might be on history files. But the prognostic state is not affected. At least that's my interpretation.

[apcraig]

For the record, test results are here,

https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#666468075b346e2da536c40dc19d964ef2c1c107
https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#5b0b3fd89dd9c7109bdd487ffdaf8b47c13c7d7e

gbox128 is not bit-for-bit with past weekend results due to merge this week.