-
Notifications
You must be signed in to change notification settings - Fork 62
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
+REMAP_AUX needs at least one more halo update. #496
Conversation
kshedstrom
commented
Oct 6, 2023
- Only when REMAP_AUXILIARY_VARS is true.
- Addresses OBC problem in tangential_vel #495
- There is still a problem with diffu and diffv when REMAP_AUXILIARY_VARS is true. The first call to horizontal_viscosity after the first REMAP returns with a tile boundary mismatch.
- This one is for CS%u_av, CS%v_av, which need to be updated coming into step_MOM_dyn_split_RK2.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #496 +/- ##
============================================
- Coverage 37.82% 37.82% -0.01%
============================================
Files 270 270
Lines 78340 78350 +10
Branches 14503 14507 +4
============================================
+ Hits 29636 29638 +2
- Misses 43300 43307 +7
- Partials 5404 5405 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this 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 halo updates are necessary and correct.
As written, there will be two separate points of synchronization for the two pass vector calls. This could be reduced to a single point of synchronization (and made more efficient) by adding the argument complete=.false.
on the first call and (optionally) complete=.true.
on the second, or they could be combined into a group_pass.
Thanks for the comments, @Hallberg-NOAA. I will submit a fix. Meanwhile, I will also attempt a fix to the diffu issue, which is due to needing a halo update on r[xy]_normal after the call to remap_OBC_fields. |
- This fixes the Bering ORLANSKI OBCs for differing processor counts. - This is either the wrong way to do group_pass for OBLIQUE OBC's or there is more wrong with them.
- Problem is in tangential_vel at tile boundaries. It matches right at the boundary, but needs some halo points to match too.
- Without this, u_av and v_av don't update a wide enough halo to get answers to reproduce across different proocessor counts with oblique OBCs.
@kshedstrom Is this ready to go in? Or did you want to continue working on it? |
You can merge this now. The Bering domain now behaves, but Mehmet and I are seeing other lingering issues with different processor counts for both the North Atlantic and the Arctic. But I think the REMAP_AUXILIARY_VARS and the OBCs should both be good now. |
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/20915 ✔️ |