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

Add MASS_WEIGHT_IN_PGF_VANISHED_ONLY to modify mass weighting in PGF #810

Conversation

claireyung
Copy link

This PR introduces the runtime variable MASS_WEIGHT_IN_PGF_VANISHED_ONLY
which has default False. If true, then the MASS_WEIGHT_IN_PRESSURE_GRADIENT
and MASS_WEIGHT_IN_PRESSURE_GRADIENT_TOP effect of weighting T/S integrals
in slanted grid cell FV PGF calculation is turned off if both sides of the
grid cell are nonvanished, where nonvanished means thickness greater than
RESET_INTXPA_H_NONVANISHED which defaults to 1e-6 m. Since the benefit of
MASS_WEIGHT_IN_PRESSURE_GRADIENT happens in vanished layers (creating a
fake PGF away from vanished layer, which is arrested by upwinded viscosity)
the benefit is still there, but now we can use MASS_WEIGHT_IN_PRESSURE_GRADIENT
for coordinates that also have slanted layers in the open ocean that are
not vanished, e.g. sigma coordinates or SIGMA_SHELF_ZSTAR coordinates in the
ice shelf where we DO trust T and S values. Additionally, this is required
near a grounding line in a 3D z-coord ice shelf as some strange looking
slanted non-vanished cells can emerge, and MWIPG being on would create fake PGFs
in non-vanished cells (and therefore generating spurious currents).

Recommend MASS_WEIGHT_IN_PGF_VANISHED_ONLY to be set to True, as well as
MASS_WEIGHT_IN_PRESSURE_GRADIENT and MASS_WEIGHT_IN_PRESSURE_GRADIENT_TOP
if you have vanishing layers with min thickness < 0.1m.

Also modifies MassWt_u and MassWt_v diagnostics to reflect usage
of MASS_WEIGHT_IN_PRESSURE_GRADIENT.

This commit should not change default answers since it defaults to False. However,
my implementation is not very efficient because it involves a lot of comparing
thicknesses and should probably be optimised.

I would appreciate suggestions to optimise this code!!

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.

Thank you for this new contribution. I appreciate where this is going, but I believe that there are dimensionally inconsistent expressions that need to be revisited, as identified in specific comments at the relevant lines. To evaluate the dimensional consistency of these expressions, you should verify that different values of H_RESCALE_POWER and Z_RESCALE_POWER give identical results.

There is also some inconsistent indenting in a few places.

Once these problems are corrected, I will be happy to accept this PR.

@claireyung claireyung marked this pull request as draft February 3, 2025 22:54
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

Attention: Patch coverage is 21.97802% with 71 lines in your changes missing coverage. Please review.

Project coverage is 38.13%. Comparing base (83efb99) to head (63fedcd).
Report is 34 commits behind head on dev/gfdl.

Files with missing lines Patch % Lines
src/core/MOM_density_integrals.F90 18.51% 58 Missing and 8 partials ⚠️
src/core/MOM_PressureForce_FV.F90 50.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #810      +/-   ##
============================================
- Coverage     38.15%   38.13%   -0.02%     
============================================
  Files           296      296              
  Lines         87249    87400     +151     
  Branches      16284    16339      +55     
============================================
+ Hits          33287    33331      +44     
- Misses        47975    48067      +92     
- Partials       5987     6002      +15     

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

@claireyung
Copy link
Author

Thanks Bob! I think I have added your suggested changes. I tested the code on the sigma seamount, showing that

  1. we can now use MASS_WEIGHT_IN_PRESSURE_GRADIENT for sigma seamount
  2. I got the same answers (with the fix) using H_RESCALE_FACTOR and Z_RESCALE_FACTOR = +/- 140 for both Boussinesq and non-Boussinesq versions. I did this by comparing output of runs: https://gist.github.com/claireyung/c80d3b44bc1861fbdf20c180bcf73586

Please let me know if there are any further changes or verification I should be doing! Thanks

@claireyung claireyung marked this pull request as ready for review February 6, 2025 23:44
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.

This PR has now addressed all of the concerns that I had with the original version, and I am pleased to accept it. Thank you for this contribution.

This commit introduces the runtime variable `MASS_WEIGHT_IN_PGF_VANISHED_ONLY`
which has default False. If true, then the `MASS_WEIGHT_IN_PRESSURE_GRADIENT`
and `MASS_WEIGHT_IN_PRESSURE_GRADIENT_TOP` effect of weighting T/S integrals
in slanted grid cell FV PGF calculation is turned off if both sides of the
grid cell are nonvanished, where nonvanished means thickness greater than
`RESET_INTXPA_H_NONVANISHED` which defaults to 1e-6 m. Since the benefit of
`MASS_WEIGHT_IN_PRESSURE_GRADIENT` happens in vanished layers (creating a
fake PGF away from vanished layer, which is arrested by upwinded viscosity)
the benefit is still there, but now we can use `MASS_WEIGHT_IN_PRESSURE_GRADIENT`
for coordinates that also have slanted layers in the open ocean that are
not vanished, e.g. sigma coordinates or SIGMA_SHELF_ZSTAR coordinates in the
ice shelf where we DO trust T and S values. Additionally, this is required
near a grounding line in a 3D z-coord ice shelf as some strange looking
slanted non-vanished cells can emerge, and MWIPG being on would create fake PGFs
in non-vanished cells (and therefore generating spurious currents).

Reccommend `MASS_WEIGHT_IN_PGF_VANISHED_ONLY` to be set to True, as well as
`MASS_WEIGHT_IN_PRESSURE_GRADIENT` and `MASS_WEIGHT_IN_PRESSURE_GRADIENT_TOP`
if you have vanishing layers with min thickness < 0.1m.

Also modifies MassWt_u and MassWt_v diagnostics to reflect usage
of MASS_WEIGHT_IN_PRESSURE_GRADIENT.

This commit should not change answers since it defaults to False. However,
my implementation is not very efficient and should probably be optimised.
…eplacing Boussinesq rho in nonBoussinesq code, and removing white space to follow 2-space indenting.
@Hallberg-NOAA Hallberg-NOAA force-pushed the dev/gfdl-jan21_2025+mwipg_vanished_only branch from 63fedcd to 6630441 Compare February 13, 2025 08:25
@Hallberg-NOAA
Copy link
Member

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

@Hallberg-NOAA Hallberg-NOAA merged commit 84a7bc0 into NOAA-GFDL:dev/gfdl Feb 13, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants