Skip to content

Mask table fix#531

Closed
kshedstrom wants to merge 3 commits into
NOAA-GFDL:dev/gfdlfrom
ESMG:mask_table_fix
Closed

Mask table fix#531
kshedstrom wants to merge 3 commits into
NOAA-GFDL:dev/gfdlfrom
ESMG:mask_table_fix

Conversation

@kshedstrom
Copy link
Copy Markdown

@kshedstrom kshedstrom commented Dec 6, 2023

No description provided.

marshallward and others added 3 commits November 15, 2023 15:26
@marshallward
Revert post_data fix to CFC concentration
f4c95ec 
The most recent NCAR -> GFDL merge created an error (courtesy of myself)
which left the CFC concentration units in the post_data call, even
though these are now handled at registration.

This patch restores this expression and removes the unit conversion from
post_data.
@kshedstrom
Merge remote-tracking branch 'gfdl/dev/gfdl' into dev/esmg
b54a72e
@kshedstrom
Fix for an OBC issue with mask_tables
cbbb16c 
 - Without this, if part of your OBC is filled with land mask and
   if that land mask contains a masked out tile, you will generate
   a NaN from the phase speed calculation where h is negative in the
   halo neighbor of that masked tile.
@kshedstrom
Copy link
Copy Markdown
Author

kshedstrom commented Dec 6, 2023 

 - Without this, if part of your OBC is filled with land mask and
   if that land mask contains a masked out tile, you will generate
   a NaN from the phase speed calculation where h is negative in the
   halo neighbor of that masked tile.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 6, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (11759d6) 37.49% compared to head (cbbb16c) 37.55%.

Files Patch % Lines
src/tracer/MOM_CFC_cap.F90 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##           dev/gfdl     #531      +/-   ##
============================================
+ Coverage     37.49%   37.55%   +0.06%     
============================================
  Files           271      271              
  Lines         79533    79401     -132     
  Branches      14816    14792      -24     
============================================
- Hits          29819    29818       -1     
+ Misses        44175    44043     -132     
- Partials       5539     5540       +1

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

Hallberg-NOAA
Hallberg-NOAA reviewed Dec 13, 2023
View reviewed changes
Copy link
Copy Markdown
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.

The changes to MOM_open_boundary here seem sensible enough and are well explained in the discussion.

I don't understand why the MOM_CFC_cap fix is showing up in this PR. It is correct, but I would think that it should appear in a separate PR to MOM6. @marshallward, do you understand how or why the MOM_CFC_cap fix is here?

I would be happy to approve this once we understand whether we should be accepting the MOM_CFC_cap fix as a part of this PR.

@kshedstrom
Copy link
Copy Markdown
Author

kshedstrom commented Dec 13, 2023

Sorry, can you pull it out?

@marshallward
Copy link
Copy Markdown
Member

marshallward commented Dec 13, 2023

@Hallberg-NOAA That commit went to the GFDL->main candidate branch. We need to remove it from this PR.

@marshallward
Copy link
Copy Markdown
Member

marshallward commented Dec 13, 2023

@kshedstrom Not easily, but I can remove it if I do a command-line merge. Seems like this has otherwise been approved so I will do that.

@marshallward
Copy link
Copy Markdown
Member

marshallward commented Dec 19, 2023

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/21708 ✔️ (Parameters are not out of date, despite warnings).

marshallward
marshallward approved these changes Dec 19, 2023
View reviewed changes
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.

Approved OBO @Hallberg-NOAA

@marshallward
Copy link
Copy Markdown
Member

marshallward commented Dec 19, 2023

This has been merged to dev/gfdl. The hashes have changed, so I have to manually close this PR.

This was referenced May 16, 2024
Closed
Closed
@marshallward marshallward mentioned this pull request May 31, 2024
Merged
@kshedstrom kshedstrom deleted the mask_table_fix branch November 3, 2024 23:18
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 left review comments

@marshallward marshallward marshallward approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kshedstrom @marshallward @Hallberg-NOAA