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 bug in kappa shear viscosity with vertex shear option. #824

Merged

Conversation

breichl
Copy link

@breichl breichl commented Feb 3, 2025

  • Viscosities at vertices along the coast were incorrectly zero'd out. This commit removes that mask so the non-zero shear driven viscosities can be interpolated from in the model. This bug caused diffusivities to be very large in channels and potentially near coastlines.
  • A bugfix flag is added with an option to use the current behavior for legacy purposes.

Here is a sample shear driven diffusivity and viscosity at 50m depth in the Gibraltar region from a case with the bug flag as True:
image

Compared to with the fix:
image

@breichl breichl requested a review from Hallberg-NOAA February 3, 2025 16:48
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 agree with these changes. I have also examined the code that was not changed, and I have convinced myself that this will not be setting non-zero values for non-coastal land points.

@Hallberg-NOAA Hallberg-NOAA force-pushed the vertex_shear_viscosity_bug branch from c0b6cd5 to 102867c Compare February 4, 2025 17:35
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.

Please change the logging of the new runtime parameter VERTEX_SHEAR_VISCOSITY_BUG so that is only logged when it is relevant, as described in the comment following the relevant line of code.

@breichl
Copy link
Author

breichl commented Feb 5, 2025

Please change the logging of the new runtime parameter VERTEX_SHEAR_VISCOSITY_BUG so that is only logged when it is relevant, as described in the comment following the relevant line of code.

Done, thanks for this suggestion.

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.

Thank you for catching this bug and offering a correction.

brandon.reichl added 3 commits February 5, 2025 13:59
- Viscosities at vertices along the coast were incorrectly zero'd out.  This commit removes that mask so the non-zero shear driven viscosities can be interpolated from in the model.  This bug caused diffusivities to be very large in channels and potentially near coastlines.
- A bugfix flag is added with an option to use the current behavior for legacy purposes.
@breichl breichl force-pushed the vertex_shear_viscosity_bug branch from a8a9bb5 to aac354c Compare February 5, 2025 19:01
@Hallberg-NOAA
Copy link
Member

This has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26303 with the expected changes in some MOM_parameter_doc files due to a new runtime parameter.

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.

2 participants