-
Notifications
You must be signed in to change notification settings - Fork 65
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
Conversation
There was a problem hiding this 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.
There was a problem hiding this 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.
9850319
to
7e00daa
Compare
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. |
This PR fixes a bug in Boussinesq finite volume pressure gradient force that
GV%Rho0
, mean seawater density andCS%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 whereRHO_PGF_REF
is set different fromRHO_0
, otherwise, this bug would have been detected. So the fix here is just for future-proof purpose.RHO_PGF_REF_BUG
is added.CS%Rho0
is renamed toCS%rho_ref
for clarification.GxRho_ref
is added to distinguish fromGxRho0
.GxRho_ref
is used to offset reference density in interface heights inpa
. AndGxRho0
is used to calculate surface pressure. PreviouslyGxRho0
was incorrectly using reference densityrho_ref
(i.e.CS%Rho0
).rho_ref
instead ofGV%rho0
. Two local variablesrho0_int_density
andrho0_set_pbce
are added to maintain and fix this issue.This PR does not change answers.
054354f: Remove unused variable
I_Rho
in threeint_density_dz
subroutines93a86dc: Add runtime flag
RHO_PGF_REF_BUG