Skip to content
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

Merged

Conversation

theresa-morrison
Copy link

@theresa-morrison theresa-morrison commented Apr 14, 2023

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.

src/ice_model.F90 Outdated Show resolved Hide resolved
@theresa-morrison
Copy link
Author

A new commit has been added to address the comments from the review.

@theresa-morrison theresa-morrison force-pushed the corrected_interspersed branch from e7a8723 to 0d18333 Compare June 9, 2023 14:46
@marshallward
Copy link
Member

marshallward commented Jun 12, 2023

This PR makes changes to the SIS2 API which causes issues with the current coupler. We will meet to discuss how to address this.

@theresa-morrison
Copy link
Author

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.
In the ice_model_init, if both the concurrent_ice (old flag) and split_fast_slow (new flag) are present in the call, the split_fast_slow setting is used and warnings are given.

@theresa-morrison
Copy link
Author

When testing, I found that an additional halo update is required which has been added in SIS_dyn_trans.F90

@kshedstrom
Copy link
Contributor

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.

@theresa-morrison
Copy link
Author

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.

@Hallberg-NOAA
Copy link
Member

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.

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a 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.

@kshedstrom
Copy link
Contributor

Yes, I approve this PR.

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).
@Hallberg-NOAA
Copy link
Member

This updated PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/SIS2/-/pipelines/20132 .

@Hallberg-NOAA Hallberg-NOAA merged commit 0006cd2 into NOAA-GFDL:dev/gfdl Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants