Skip to content

(*)tidal_bay_set_OBC_data works with grid rotation#908

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

(*)tidal_bay_set_OBC_data works with grid rotation#908
Hallberg-NOAA merged 1 commit into
NOAA-GFDL:dev/gfdlfrom
Hallberg-NOAA:tidal_bay_OBC_rotation

Conversation

@Hallberg-NOAA
Copy link
Copy Markdown
Member

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

Modified tidal_bay_set_OBC_data to work properly with grid rotation of 90, 180 or 270 degrees. With these changes the ESMG Tidal_bay 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 29, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2025

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 37.49%. Comparing base (7334c8c) to head (6684aaf).

Files with missing lines Patch % Lines
src/user/tidal_bay_initialization.F90 0.00% 29 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #908      +/-   ##
============================================
- Coverage     37.50%   37.49%   -0.01%     
============================================
  Files           306      306              
  Lines         91794    91814      +20     
  Branches      17579    17584       +5     
============================================
+ Hits          34424    34425       +1     
- Misses        50930    50950      +20     
+ Partials       6440     6439       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Hallberg-NOAA Hallberg-NOAA force-pushed the tidal_bay_OBC_rotation branch from 6684aaf to 4911381 Compare June 6, 2025 22:57
@kshedstrom
Copy link
Copy Markdown

I compiled a code with your three PRs all included since there are no conflict between them. For Tidal bay I got:

Backtrace for this error:
#0  0x7f073d1ac98f in ???
#1  0x4edb47 in __mom_MOD_initialize_mom
	at //import/c1/AKWATERS/kate/ESMG/ESMG-configs/src/MOM6/src/core/MOM.F90:3118
#2  0x446bc7 in mom6
	at //import/c1/AKWATERS/kate/ESMG/ESMG-configs/src/MOM6/config_src/drivers/solo_driver/MOM_driver.F90:280
#3  0x4072ac in main
	at //import/c1/AKWATERS/kate/ESMG/ESMG-configs/src/MOM6/config_src/drivers/solo_driver/MOM_driver.F90:27

The line in question is:

if (any(CS%OBC%tracer_x_reservoirs_used)) then

I see that this array is never allocated when there are no tracers. Tidal bay has no tracers.

@kshedstrom
Copy link
Copy Markdown

I will approve once the no tracers case is handled (see my fix in the shelfwave PR).

@Hallberg-NOAA
Copy link
Copy Markdown
Member Author

Hallberg-NOAA commented Jun 9, 2025

I have updated PR #895 to address this problem with an unassociated being referenced with the following simple fix to MOM.F90:

@@ -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?

  Modified tidal_bay_set_OBC_data to work properly with grid rotation of 90, 180
or 270 degrees.  With these changes the ESMG Tidal_bay test case is now giving
bitwise identical answers under grid rotation.  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 tidal_bay_OBC_rotation branch from 4911381 to 995ab97 Compare June 9, 2025 19:10
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.

OBO @kshedstrom
(issue raised is due to #895)

@Hallberg-NOAA
Copy link
Copy Markdown
Member Author

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

@Hallberg-NOAA Hallberg-NOAA merged commit 756b2d4 into NOAA-GFDL:dev/gfdl Jun 10, 2025
52 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the tidal_bay_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