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

Set flags properly for rotated tripolar grids #581

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

Hallberg-NOAA
Copy link
Member

Added code to tht two versions of clone_MD_to_MD in the FMS1 and FMS2 versions of MOM_domain_infra.F90 to properly change the flags when a tripolar grid is rotated, so that it does not lead to misleadingly incorrect answers. With the current versions of FMS, doing grid rotation testing with a tripolar grid will lead to error messages about the incomplete implementation of FMS, but these failures are preferable to the model silently working incorrectly. All answers are bitwise identical in cases that worked correctly before.

@Hallberg-NOAA Hallberg-NOAA added the bug Something isn't working label Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.12%. Comparing base (2b59089) to head (d9bd123).

Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #581      +/-   ##
============================================
- Coverage     37.13%   37.12%   -0.01%     
============================================
  Files           271      271              
  Lines         80779    80794      +15     
  Branches      15068    15082      +14     
============================================
- Hits          29999    29998       -1     
- Misses        45181    45195      +14     
- Partials       5599     5601       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marshallward
Copy link
Member

I am confused about the interpretation here. Generally turns is used to indicate a clockwise rotation from input grid to model grid. If MD_in has its north Y_AXIS folded and we rotate once, then the MOM_dom fold should be on the east face, i.e. MOM_dom%X_AXIS = FOLD_EAST_EDGE should have the fold on the east side. But here we set X_AXIS to FOLD_WEST_AXIS. Or is the operation backwards here?

The two-turn case looks correct to me.

Also, there are if modulo(qturns, 2) blocks preceding these if modulo(qturns, 4) = 1, 2, 3 blocks. These statements could be divided up and moved inside of the modulo(qturns, 2) blocks.

@Hallberg-NOAA
Copy link
Member Author

The main point of this PR was to get FMS to issue a message indicating that it has not yet fully implemented rotated tripolar grids, rather than just silently doing completely the wrong thing when we tried to put in turns with a tripolar grid. You would be better positioned to describe the sense of the turns, @marshallward, than I am, and I am sure that you are right about the changes we need to make, and we should make them now. The true test, though, will be when FMS finally supports southern-, eastern- and western- tripolar grids, and then we can actually see what gives the right answers!

  Added code to tht two versions of clone_MD_to_MD in the FMS1 and FMS2 versions
of MOM_domain_infra.F90 to properly change the flags when a tripolar grid is
rotated, so that it does not lead to misleadingly incorrect answers.  With the
current versions of FMS, doing grid rotation testing with a tripolar grid will
lead to error messages about the incomplete implementation of FMS, but these
failures are preferable to the model silently working incorrectly.  All answers
are bitwise identical in cases that worked correctly before.
@Hallberg-NOAA
Copy link
Member Author

This PR has been updated to make the changes suggested in the review.

Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/22871 ✔️

@marshallward marshallward merged commit 5e34f48 into NOAA-GFDL:dev/gfdl Apr 1, 2024
12 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the Tripolar_rotation branch May 10, 2024 21:29
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