-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update BGC and Icepack interfaces #968
Conversation
Update interfaces and bgc settings consistent with new version of Icepack BGC.
Update a few bgc test to increase diagnostics.
Ran a full test suite on Derecho with intel, cray, gnu compilers. All cases are bit-for-bit except the bgc cases (as expected). In addition, 31 of 60 bgc test cases do not run. 14/18 skl cases fail. 18/20 cray cases fail. 1/20 non-skl case fails with intel and 2/20 non-skl cases fail with gnu.
|
Refactor bgc_tracer_type initialization in CICE ice_init_column.F90, subroutine init_zbgc, fixes implementation bug
Resynchronize unittest source code as needed (opticep/ice_init_column.F90)
Several changes have been made to CICE and Icepack in the last couple weeks. Icepack still needs to be updated, but a full test suite was run on Derecho with 5 and 6 compilers for CICE and Icepack respectively on Sept 12. Test results are as expected. All tests are bit-for-bit except bgc and congel. A small number of tests do fail but that's consistent with what is happening on the main trunk. These failed tests are understood (mostly hdf5 and nvhpc compiler). I think the Icepack PR and this PR are ready to be merged if approved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one edit needed in ice_in. Do you expect the congel test to be non-BFB?
configuration/scripts/ice_in
Outdated
grid_o = 0.006 | ||
grid_o_t = 0.006 | ||
l_sk = 0.024 | ||
l_sk = 20.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l_sk=2.0 in Icepack and in the CICE user guide
I fixed the default namelist. Also, yes, the congel tests are changing answers relative to the current main. But they are only different by roundoff. This was done to preserve bit-for-bit with non-bgc and non-congel tests. From my testing, the bgc and congel changes separately preserved bit-for-bit, but when done together, they changed results (for non-bgc, non-congel cases). This must have been happening due to some compiler optimization. I refactored the congel code slightly which changes congel results to roundoff, but then the compiler ended up retaining bit-for-bit when the congel and bgc changes were both introduced for non-congel, non-bgc cases. Clear as day? I plan to merge this tonight after some final testing. |
I merged the BGC changes. Thanks @njeffery for all your help. I know working towards merging in the Consortium versions was some extra effort. Derecho is down this week, but I will retest the merged version as soon as I can. I suspect we may have a few minor things to fix down the track, but I actually think we're in quite good shape overall at this point! |
Excellent! well done @apcraig! |
This reverts commit d9d0176.
PR checklist
Update BGC
njeffery, eclare108213, apcraig
Ran full test suite on Derecho with 5 compilers. Results are as expected. skl tests are removed, zaero tests are added.
Deprecate skl BGC but leave code alone for now hoping we get help from the community to validate the latest code.
Update interfaces and bgc settings consistent with new version of Icepack BGC.
Remove redundant arguments in non-BGC interfaces.
Generalize merge_fluxes to make all arguments optional
Fix bug in subroutine snow_redist computation of hsn_new when nslyr=1
Update icepack_init_zbgc call.
Update bgc namelist defaults and settings in ice_in
Update testing, remove skl tests, add zaero tests.
We still need to