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

Bugfix for mixing up Rho0 and rho_ref in Boussinesq PGF #837

Merged
merged 3 commits into from
Feb 24, 2025

Conversation

herrwang0
Copy link

@herrwang0 herrwang0 commented Feb 19, 2025

This PR fixes a bug in Boussinesq finite volume pressure gradient force that GV%Rho0, mean seawater density and CS%Rho0, reference density are incorrectly interchanged in a number of instances. The bug probably has zero effect on any existing runs, as I am unaware of cases where RHO_PGF_REF is set different from RHO_0, otherwise, this bug would have been detected. So the fix here is just for future-proof purpose.

  • A new runtime parameter RHO_PGF_REF_BUG is added.
  • CS%Rho0 is renamed to CS%rho_ref for clarification.
  • A new local variable GxRho_ref is added to distinguish from GxRho0. GxRho_ref is used to offset reference density in interface heights in pa. And GxRho0 is used to calculate surface pressure. Previously GxRho0 was incorrectly using reference density rho_ref (i.e. CS%Rho0).
  • Subroutine calls for density integral and baroclinic pressure force pbce were incorrectly using rho_ref instead of GV%rho0. Two local variables rho0_int_density and rho0_set_pbce are added to maintain and fix this issue.

This PR does not change answers.

054354f: Remove unused variable I_Rho in three int_density_dz subroutines
93a86dc: Add runtime flag RHO_PGF_REF_BUG

@herrwang0 herrwang0 added bug Something isn't working Parameter change Input parameter changes (addition, removal, or description) labels Feb 20, 2025
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.

Having carefully examined these proposed changes, I am convinced that they are correct. In particular, any choosing any value of RHO_PGF_REF should give mathematically equivalent answers, and these changes should recover this property.

However, in examining these changes, I think that I have spotted a separate bug related to a missing reference geopotential height (G%Z_ref) correction. Please take a look at the relevant line, @herrwang0, and let me know what you think. In particular, answers should be mathematically equivalent when different values of REFERENCE_HEIGHT are used. I leave it up to you, though, @herrwang0, to decide whether this additional bug (if it does exist) should be dealt with in this same PR or whether it should be dealt with separately. Note that in every case I could find, REFERENCE_HEIGHT = 0.0, so such a bug seems very unlikely to change answers.

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.

With the recent minor update, I think that everything in this PR looks correct. Thank you for this valuable contribution.

I_Rho = 1.0/rho_0 is never used and therefore removed.
This new parameter addresses a bug in Boussinesq finite volume pressure
gradient forces (MOM_PressureForce_FV) that the mean seawater density
Rho_0 and reference density Rho_ref are used incorrectly in several
instances. It should be noted that by default Rho_ref is Rho_0 and
Rho_ref is rarely set to other than Rho_0.
Recover reference height offset (Z_ref) in calculating bottom pressure
for self-attraction and loading in Boussinesq mode.
@Hallberg-NOAA
Copy link
Member

This PR has passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/26508 with the expected warnings about a new runtime parameter.

@Hallberg-NOAA Hallberg-NOAA merged commit 5f23058 into NOAA-GFDL:dev/gfdl Feb 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Parameter change Input parameter changes (addition, removal, or description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants