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.

Sign up for GitHub

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

Jump to bottom

Fix issue 19 porous face oob #20

Merged
merged 2 commits into from
Dec 1, 2021
Merged

Fix issue 19 porous face oob #20

merged 2 commits into from
Dec 1, 2021

Conversation

adcroft
Copy link
Member

@adcroft adcroft commented Dec 1, 2021

Fixes an out-of-bounds k-index in MOM_continuity_PPM.F90 introduced with the porous topography update. Problem reported in #19

Two commits:

Needs testing/review by @kshedstrom

@adcroft
Cleaned up whitespace leftover from porous topomerge.
d73870f 
- Spacing within expressions was uneven and made multiplation look like
  POW functions. Leftover from merging NOAA-GFDL#3.
- No answer changes.
@adcroft
Fix out-of-bounds k index in PPM flux
4f2b1e0 
- An errant use of the porous face area led to an out-of-bounds k-index
  reported in NOAA-GFDL#19.
- Closes NOAA-GFDL#19
@Hallberg-NOAA Hallberg-NOAA linked an issue Dec 1, 2021 that may be closed by this pull request
Recent update broke DOME #19
Closed
@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #20 (4f2b1e0) into dev/gfdl (a0248ef) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl      #20   +/-   ##
=========================================
  Coverage     29.17%   29.17%           
=========================================
  Files           240      240           
  Lines         71481    71481           
=========================================
  Hits          20852    20852           
  Misses        50629    50629
Impacted Files Coverage Δ
src/core/MOM_continuity_PPM.F90 39.38% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0248ef...4f2b1e0. Read the comment docs.

@kshedstrom
Copy link

Thanks, yes, this fixes #19.

Hallberg-NOAA
Hallberg-NOAA approved these changes Dec 1, 2021
View reviewed changes
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a 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 changes are likely to be correct, and as Kate notes, they do fix issue #19. This PR has passed TC testing, and the pipeline testing for this case is being run at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14241. This PR should be accepted on an expedited basis (superseding other PRs) as soon as the pipeline testing has successfully completed.

@marshallward
Copy link
Member

Regression has passed: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14241 ✔️

@marshallward marshallward merged commit ceb181f into NOAA-GFDL:dev/gfdl Dec 1, 2021
This was referenced Jan 29, 2022
Closed
Merged
@Hallberg-NOAA Hallberg-NOAA added the bug Something isn't working label Feb 1, 2022
@adcroft adcroft deleted the fix-issue-19-porous-face-OOB branch May 31, 2022 17:27
theresa-morrison pushed a commit to theresa-morrison/MOM6 that referenced this pull request Dec 16, 2024
@yichengt900
Added Generic Tracer Sponges (NOAA-GFDL#20)
90fc4cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hallberg-NOAA Hallberg-NOAA Hallberg-NOAA approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Recent update broke DOME
4 participants
@adcroft @kshedstrom @marshallward @Hallberg-NOAA