Skip to content

Add conditional halo pass for frazil inside make_frazil#932

Merged
Hallberg-NOAA merged 3 commits into
NOAA-GFDL:dev/gfdlfrom
breichl:pass_frazil
Jul 10, 2025
Merged

Add conditional halo pass for frazil inside make_frazil#932
Hallberg-NOAA merged 3 commits into
NOAA-GFDL:dev/gfdlfrom
breichl:pass_frazil

Conversation

@breichl
Copy link
Copy Markdown

@breichl breichl commented Jul 8, 2025

Frazil may need an extra halo pass, but only when all of the following are true:
(1) frazil is calculated and applied in the halo (e.g., vertex_shear is true)
(2) the thermodynamic loop is called more than once before passing frazil to the sea-ice (dt_therm<dt_cpld)
(3) reclaim_frazil is true
(4) a previous make_frazil was called after the diabatic loop and before T&S were updated in halos.

This commit adds the halo pass inside make_frazil since it is where we can know all of these conditions at once. We needed to add a flag to know if frazil was reset (which happens every timestep if dt_therm>=dt_cpld), in which case the halo pass would be unnecessary. We needed an optional argument for make_frazil so we could flag to only do this update on the first call to make_frazil (which is needed because the previous call to make_frazil was after the diabatic loop, condition 4).

We could have achieved the same result by adding one new halo pass in diabatic_driver right before line 376 (requiring only one line of new code), but it would unnecessarily invoke a halo pass when all the above conditions were not achieved.

Without this new code, the model does not conserve or reproduce across layouts when dt_therm<dt_cpld, vertex_shear is True, and reclaim_frazil is True.

@breichl breichl added the bug Something isn't working label Jul 8, 2025
@Hallberg-NOAA
Copy link
Copy Markdown
Member

It looks like RECLAIM_FRAZIL is true is almost every case, so I think that it would be fine to do the extra halo update without making it conditional on the setting of RECLAIM_FRAZIL. With this consideration, I think that it would be preferable to move the new halo update outside of make_frazil(), if for no other reason than to make make_frazil() more likely to work well when MOM6 is ported to work on GPUs.

@breichl
Copy link
Copy Markdown
Author

breichl commented Jul 10, 2025

It looks like RECLAIM_FRAZIL is true is almost every case, so I think that it would be fine to do the extra halo update without making it conditional on the setting of RECLAIM_FRAZIL. With this consideration, I think that it would be preferable to move the new halo update outside of make_frazil(), if for no other reason than to make make_frazil() more likely to work well when MOM6 is ported to work on GPUs.

The code was updated with this in mind, which greatly simplifies the necessary modifications.

Copy link
Copy Markdown
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 agree that these changes are correct and that they should address our recent experience with some instances where the interaction between the handling of the frazil and the vertex shear code were causing some configurations not to reproduce across processor layout.

brandon.reichl added 3 commits July 10, 2025 14:01
Frazil may need an extra halo pass, but only when all of the following are true:
(1) frazil is calculated and applied in the halo (e.g., vertex_shear is true)
(2) the thermodynamic loop is called more than once before passing frazil to the sea-ice (dt_therm<dt_cpld)
(3) reclaim_frazil is true
(4) a previous make_frazil was called after the diabatic loop and before T&S were updated in halos.

This commit adds the halo pass inside make_frazil since it is where we can know all of these conditions at once.  We needed to add a flag to know if frazil was reset (which happens if dt_therm>=dt_cpld), in which case the halo pass would be unnecessary.  We needed an optional argument for make_frazil so we could flag to only do this update on the first call to make_frazil (which is needed because the previous call to make_frazil was after the diabatic loop, condition 4)
@Hallberg-NOAA
Copy link
Copy Markdown
Member

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/28088.

@Hallberg-NOAA Hallberg-NOAA merged commit 792248c into NOAA-GFDL:dev/gfdl Jul 10, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants