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 brine diag #526

Closed
wants to merge 2 commits into from
Closed

Fix brine diag #526

wants to merge 2 commits into from

Conversation

kshedstrom
Copy link

NCAR reported that I broke something with #401. See if this fixes it?

marshallward and others added 2 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
Fix the saltFluxAdded diagnoistic, broken in NOAA-GFDL#401
7ef6a57
@codecov Codecov
Copy link

codecov bot commented Nov 21, 2023 •
edited
Loading

Codecov Report

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

Comparison is base (cce4b3d) 37.46% compared to head (7ef6a57) 37.50%.

Files Patch % Lines
src/core/MOM_forcing_type.F90 0.00% 1 Missing and 1 partial ⚠️
src/tracer/MOM_CFC_cap.F90 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##           dev/gfdl     #526      +/-   ##
============================================
+ Coverage     37.46%   37.50%   +0.04%     
============================================
  Files           270      270              
  Lines         79763    79675      -88     
  Branches      14830    14814      -16     
============================================
- Hits          29880    29879       -1     
+ Misses        44349    44260      -89     
- Partials       5534     5536       +2

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

@marshallward
Copy link
Member

This is the second mysterious FPE that I've seen in two weeks, although at least this time the FPE was turned on. Doubly strange that the backtrace has no symbols, even though it was clearly built with -g.

This is going to be hard to reproduce but I will look into it.

@marshallward marshallward mentioned this pull request Nov 22, 2023
Merged
@marshallward
Copy link
Member

Rerunning the test seems to have magically fixed the MacOS FPE errors (the worst of best outcomes), hopefully it was nothing to do with us.

But for now: This has passed local regression so I will merge this into the PR to main. (NOT into dev/gfdl, so no one should click-merge this, please!)

@marshallward
Copy link
Member

This has been merged into dev-gfdl-main-candidate-2023-11-13 so closing it now. It will cycle back into dev/gfdl after that PR has been approved.

@kshedstrom kshedstrom deleted the fix_brine_diag 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
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kshedstrom @marshallward