CICE-Consortium / Icepack

Development repository for sea-ice column physics
Other
25 stars 131 forks source link

Add saltflux option. #418

Closed dabail10 closed 1 year ago

dabail10 commented 1 year ago

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

apcraig commented 1 year ago

Please do standalone Icepack testing including regression testing.

dabail10 commented 1 year ago

I did the Icepack regression testing with intel, pgi, and gnu on cheyenne. Everything is bfb.

dabail10 commented 1 year ago

I agree. I should probably add tests for both.

apcraig commented 1 year ago

Sounds good. Maybe we can merge the Icepack PR today and the CICE PR in the next day or two then I can start testing for the release. Thanks!

apcraig commented 1 year ago

How about if we call the salt test "saltprog" instead of just "salt"? Does the new test pass?

dabail10 commented 1 year ago

If we are doing a longer name I would prefer saltflux.

dabail10 commented 1 year ago

The test passes in CICE with the updated Icepack submodule.

apcraig commented 1 year ago

saltflux is OK too. Or saltfluxprog. Can you also do a quick run with standalone Icepack for the new test. Lets just make sure it runs and passes. Then we can merge. Thanks!

dabail10 commented 1 year ago

Did quick_suite on intel.

./icepack.setup --suite quick_suite --mach cheyenne --env intel --testid saltflux4 -s saltflux

10 measured results of 10 total results 10 of 10 tests PASSED 0 of 10 tests PENDING 0 of 10 tests MISSING data 0 of 10 tests FAILED

apcraig commented 1 year ago

The new test is in the base_suite.

apcraig commented 1 year ago

OK, I see you ran quick_suite with -s saltflux. I guess that's OK too.

apcraig commented 1 year ago

You need to add set_nml.saltflux to your push.

dabail10 commented 1 year ago

Whoops. Should be there now.