Skip to content

(*)Shelfwave_set_OBC_data works with grid rotation#895

Merged
Hallberg-NOAA merged 1 commit into
NOAA-GFDL:dev/gfdlfrom
Hallberg-NOAA:shelfwave_OBC_rotation
Jun 9, 2025
Merged

(*)Shelfwave_set_OBC_data works with grid rotation#895
Hallberg-NOAA merged 1 commit into
NOAA-GFDL:dev/gfdlfrom
Hallberg-NOAA:shelfwave_OBC_rotation

Conversation

@Hallberg-NOAA
Copy link
Copy Markdown
Member

@Hallberg-NOAA Hallberg-NOAA commented May 2, 2025

Modified shelfwave_set_OBC_data() to work properly with grid rotation of 90, 180 or 270 degrees. With these changes the ESMG shelfwave test case is now giving bitwise identical answers under grid rotation. This commit also corrects a recently added problem with potentially unallocated arrays being used to test whether a pointer is set to the unrotated tracer reservoirs in cases where there are no tracers. This commit does change (and fix) answers when grid rotation is used, but answers are bitwise identical when there is no grid rotation, and there are no changes to any public interfaces.

@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working enhancement New feature or request answer-changing A change in results (actual or potential) labels May 2, 2025
@kshedstrom
Copy link
Copy Markdown

This code looks fine and I've seen that it works for @Hallberg-NOAA. However, it also depends on changes to MOM_open_boundary.F90 which I don't have, so I can't directly test this yet.

@Hallberg-NOAA Hallberg-NOAA marked this pull request as draft May 12, 2025 21:58
@Hallberg-NOAA Hallberg-NOAA force-pushed the shelfwave_OBC_rotation branch from 1b6e12c to 1ae485d Compare May 29, 2025 07:07
@Hallberg-NOAA Hallberg-NOAA marked this pull request as ready for review May 29, 2025 07:07
@Hallberg-NOAA Hallberg-NOAA force-pushed the shelfwave_OBC_rotation branch from 1ae485d to 55b1c37 Compare May 29, 2025 07:08
@Hallberg-NOAA
Copy link
Copy Markdown
Member Author

This commit is ready for review, but some of the bug fixes in PR #905 and PR #907 are beneficial to this case and should be handled first.

@Hallberg-NOAA Hallberg-NOAA force-pushed the shelfwave_OBC_rotation branch from 55b1c37 to 69bd1ed Compare June 6, 2025 22:58
@kshedstrom
Copy link
Copy Markdown

This fails like Tidal Bay does, but runs fine with this little change to MOM.F90:

diff --git a/src/core/MOM.F90 b/src/core/MOM.F90
index 989d12e37..1d97df9b8 100644
--- a/src/core/MOM.F90
+++ b/src/core/MOM.F90
@@ -3115,11 +3115,13 @@ subroutine initialize_MOM(Time, Time_init, param_file, dirs, CS, &
           OBC_in%cff_normal_u => CS%OBC%cff_normal_u
           OBC_in%cff_normal_v => CS%OBC%cff_normal_v
         endif
-        if (any(CS%OBC%tracer_x_reservoirs_used)) then
-          OBC_in%tres_x => CS%OBC%tres_x
-        endif
-        if (any(CS%OBC%tracer_y_reservoirs_used)) then
-          OBC_in%tres_y => CS%OBC%tres_y
+        if (CS%OBC%ntr > 0) then
+          if (any(CS%OBC%tracer_x_reservoirs_used)) then
+            OBC_in%tres_x => CS%OBC%tres_x
+          endif
+          if (any(CS%OBC%tracer_y_reservoirs_used)) then
+            OBC_in%tres_y => CS%OBC%tres_y
+          endif
         endif
       ! else
         ! Do we need to swap around the u- and v-components?

I approve once the case of no tracers is handled.

@Hallberg-NOAA Hallberg-NOAA force-pushed the shelfwave_OBC_rotation branch 2 times, most recently from 05e12f6 to 7167141 Compare June 9, 2025 14:56
  Modified shelfwave_set_OBC_data to work properly with grid rotation of 90, 180
or 270 degrees.  With these changes the ESMG shelfwave test case is now giving
bitwise identical answers under grid rotation.  This commit also corrects a
recently added problem with potentially unallocated arrays being used to test
whether a pointer is set to the unrotated tracer reservoirs in cases where there
are no tracers.  This commit does change (and fix) answers when grid rotation is
used, but answers are bitwise identical when there is no grid rotation, and
there are no changes to any public interfaces.
@Hallberg-NOAA Hallberg-NOAA force-pushed the shelfwave_OBC_rotation branch from 7167141 to d771cea Compare June 9, 2025 16:09
@Hallberg-NOAA
Copy link
Copy Markdown
Member Author

I have updated this PR to address this problem with an unassociated being referenced with the following even simpler fix:

@@ -3114,12 +3114,12 @@ subroutine initialize_MOM(Time, Time_init, param_file, dirs, CS, &
           OBC_in%ry_oblique_v => CS%OBC%ry_oblique_v
           OBC_in%cff_normal_u => CS%OBC%cff_normal_u
           OBC_in%cff_normal_v => CS%OBC%cff_normal_v
         endif
-        if (any(CS%OBC%tracer_x_reservoirs_used)) then
+        if (associated(CS%OBC%tres_x)) then
           OBC_in%tres_x => CS%OBC%tres_x
         endif
-        if (any(CS%OBC%tracer_y_reservoirs_used)) then
+        if (associated(CS%OBC%tres_y)) then
           OBC_in%tres_y => CS%OBC%tres_y
         endif
       ! else
         ! Do we need to swap around the u- and v-components?

Copy link
Copy Markdown
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.

@kshedstrom I'm going to tentatively approve this based on your previous feedback, but please let us know if the associated() change does not fix the issue.

@kshedstrom
Copy link
Copy Markdown

Yes, I approve the new version.

@Hallberg-NOAA
Copy link
Copy Markdown
Member Author

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

@Hallberg-NOAA Hallberg-NOAA merged commit 8c67912 into NOAA-GFDL:dev/gfdl Jun 9, 2025
52 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the shelfwave_OBC_rotation branch June 17, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

answer-changing A change in results (actual or potential) bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants