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

Bugfix in getting surface currents [Additions from GMAO (3)] #1356

Conversation

sanAkel
Copy link
Collaborator

@sanAkel sanAkel commented Mar 23, 2021

This PR fixes a bug.

In https://github.com/NOAA-GFDL/MOM6/pull/1134 subroutine ocean_model_get_UV_surf was introduced, but it lacked the needed call to pass_vector to correctly update halo regions. This is now corrected.

Thanks to @zhaobin74 for bringing it up and @Hallberg-NOAA for confirming my mistake!

@sanAkel sanAkel marked this pull request as ready for review March 23, 2021 20:12
Copy link
Collaborator

@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 this halo update is necessary. However, there is some duplication in that this routine extracts either the u or v component of the surface velocity by name, and not both. If both were extracted simultaneously, there would only need to be one halo update for both, rather than one for each of the two velocity components.

@Hallberg-NOAA
Copy link
Collaborator

@sanAkel, there have been other commits to dev/gfdl since this was added, so this PR is now out-of-date with the base branch. However, you do not appear to have given the maintainers the ability to modify this PR. Please click the "Update Branch" button so we can trigger the pipeline tests and merge this PR into dev/gfdl.

@sanAkel
Copy link
Collaborator Author

sanAkel commented Mar 27, 2021

@Hallberg-NOAA, appreciate the vector pair suggestion. I intend to make some changes to our coupler (making use of new infrastructure), will certainly include it in that, just wanted to get the bug fix out of my way first! Thanks for the suggestion (also updated the branch, not sure how I would let you all do it for me).

@Hallberg-NOAA
Copy link
Collaborator

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

@Hallberg-NOAA Hallberg-NOAA merged commit 13f1e70 into mom-ocean:dev/gfdl Mar 28, 2021
@sanAkel sanAkel deleted the bugfix/sanAkel/output-b-grid-currents branch March 28, 2021 17:23
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