-
Notifications
You must be signed in to change notification settings - Fork 44
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
Correct interspersed option in the CIOD #194
Correct interspersed option in the CIOD #194
Conversation
A new commit has been added to address the comments from the review. |
e7a8723
to
0d18333
Compare
This PR makes changes to the SIS2 API which causes issues with the current coupler. We will meet to discuss how to address this. |
After discussing how to correct the API issue with Marshall and Bob, I changed the order of the optional arguments and removed the changes to existing variable names. The new versions should compile and run with the current coupler. |
When testing, I found that an additional halo update is required which has been added in SIS_dyn_trans.F90 |
This code calls open on SIS_input twice, not closing the file between open calls. gfortran complains with a fatal error. Also, sooner or later you will get scolded for trailing white space. I found it best to teach my editor to make them go away on saving a file. |
Thanks for testing this, Kate. I think I fixed the issue if you want to try it again and eliminated most of the trailing white space. |
I have evaluated these changes in the existing MOM6-examples test suite, and they do not change any answers in any existing cases, including coupled_AM2_LM3_SIS2/Intersperse_ice_1deg, because it is using the default (true) value of USE_INTERSPERSE_BUG. |
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.
I have examined all of the changes in this PR, and they all make sense to me.
This PR has passed the pipeline testing at gitlab.gfdl.noaa.gov/ogrp/SIS2/-/pipelines/20128, with the expected warning about a new runtime parameter in the CIOD_parameter_doc.all files.
Yes, I approve this PR. |
c2057f2
to
87444fb
Compare
This commit includes the changes to the combined ice ocean driver and a few other files to (1) allow the combined ice ocean driver to run with a data atmosphere and (2) correct the interspersed coupling option so that the information from the updated ocean state is communicated to the sea-ice. A new flag has been added to the combined ice ocean driver: use_intersperse_bug which recovers the answers from the version with the incorrect coupling. By default, the bug is used. This is because the Ocean_ice_boundary_type is a nessecary input for the update_slow_ice_and_ocean subroutine in order to get the correct coupling. Changes to ice_model, SIS_dyn_trans, and ice_type are in order to run the combined ice ocean driver with a data atmosphere. In that case, the slow and fast sea ice PEs are specified but technically the same PE lists. In ice_model_init the "concurrent" flag has been renamed to "split_fast_slow" but it is still called "Concurrent_ice" in the argument list. A halo update was needed for the sea ice mass when multiple interspesed cycles are used per coupling time step(DT_CIOD<dt_cpld).
87444fb
to
b5b178c
Compare
This updated PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/SIS2/-/pipelines/20132 . |
Changes to the combined ice-ocean driver (CIOD) to fix a bug. The state of the ocean is now passed to the ice through the OIB (ocean ice boundary type). A new subroutine has been added to combined_ice_ocean_driver.F90: direct_flux_ocn_to_OIB which is analogous to the existing direct_flux_ice_to_IOB subroutine. The new subroutine assumes the ocean and ice are on the same model grid and that the do_area_weighted_flux option is false for the frazil ice.
This new version of the CIOD requires the OIB as an input when update_slow_ice_and_ocean is called. Currently it is an optional argument, but there is a fatal error if the CIOD is used without it. Therefore, to use the corrected version of the CIOD the call to update_slow_ice_and_ocean in coupler_main must be modified.
The ice_model_init routine is changed in two ways. First, the concurrent atmosphere and concurrent ice options are now both inputs so they are no longer assumed to be the same. Second, there are minor changes to the slow and fast processor calls so the slow/fast processors can be shared.
The direct_flux_ocn_to_OIB subroutine uses the unpack_ocn_ice_bdry subroutine from the ice_model module. The routine is changed to public and the use statement is added to the CIOD.
This PR will change answers for the coupled_AM2_LM3_SIS2/Intersperse_ice_1deg simulation, but should not change the solutions of any setup that does not use the interspersed option of the combined ice-ocean driver. The coupled_AM2_LM3_SIS2/Intersperse_ice_1deg simulation was run with a slightly modified version of the Xanadu coupler. Note that answers will only change when DT_COUPLED_ICE_OCEAN_DYN is less than dt_cpld. The USE_INTERSPERSE_BUG flag can be used to recover the old version.