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

GNU bug fix for MOM_variables.F90 #1605

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

mathomp4
Copy link
Collaborator

@mathomp4 mathomp4 commented Aug 2, 2023

Upon the update of MOM6 in GEOS-ESM to the latest code, our CI GNU Debug testing threw a failure:

https://app.circleci.com/pipelines/github/GEOS-ESM/GEOSgcm/2849/workflows/cb295ace-751c-439c-9ad4-b1922a7c4cc7/jobs/5573/parallel-runs/0/steps/0-105

Tracing it back to line 1390 in src/parameterizations/vertical/MOM_diabatic_driver.F90

if (associated(visc%sfc_buoy_flx)) then
visc%sfc_buoy_flx(:,:) = SkinBuoyFlux(:,:)
call pass_var(visc%sfc_buoy_flx, G%domain, halo=1)
endif

Experience with GNU tells us that GNU does not handle pointers that aren't nullified and not associated well. So associated(pointer) could be true, could be false, or unknown. I think the Fortran Standard says it's an undefined state.

The fix is simple though. Null the pointer in the derived type declaration. It also matches all the other pointers which are that way.

(For reference, this change came in via #1603 through NOAA-GFDL#352 it looks like.)

@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #1605 (f39e7f7) into main (b61b29e) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head f39e7f7 differs from pull request most recent head 82acb2f. Consider uploading reports for the commit 82acb2f to get more accurate results

@@           Coverage Diff           @@
##             main    #1605   +/-   ##
=======================================
  Coverage   38.16%   38.17%           
=======================================
  Files         269      269           
  Lines       76503    76503           
  Branches    14064    14064           
=======================================
+ Hits        29200    29203    +3     
+ Misses      42056    42052    -4     
- Partials     5247     5248    +1     
Files Changed Coverage Δ
src/core/MOM_variables.F90 46.96% <ø> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@kshedstrom kshedstrom left a comment

Choose a reason for hiding this comment

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

I approve this PR.

Copy link
Collaborator

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

GFDL approves this change, and apologies for letting it slip through.

We will look over our testing procedure and try to apply any improvements (with -fcheck=all being the most apparent).

@jiandewang
Copy link
Collaborator

works fine in EMC

@marshallward
Copy link
Collaborator

Reminder to @alperaltuntas / @gustavo-marques, @abozec it review this, it should be relatively simple.

Copy link
Collaborator

@abozec abozec left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I was out of the country. COAPS approves.

@marshallward
Copy link
Collaborator

Thanks all, and remember to update your forks!

@marshallward marshallward merged commit 2dd99f8 into mom-ocean:main Aug 11, 2023
10 checks passed
@marshallward
Copy link
Collaborator

marshallward commented Aug 11, 2023

(Also this quasirandom fail should be fixed in the next dev/gfdl -> main update)

https://github.com/mom-ocean/MOM6/actions/runs/5833705621/job/15821701926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants