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 PPM_CM and HYCOM1_ONLY_IMPROVES #341

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

awallcraft
Copy link

Add "PPM_CW" as an option for INTERPOLATION_SCHEME and REMAPPING_SCHEME. This implements the original Colella and Woodward (1984) edge calculation for PPM. It computes 4th order explicit edge values but constrains them to produce a monotonic profile, which is particularly effective for remapping.

INTERPOLATION_SCHEME="PPM_CW" is identical to "REMAPPING_PPM_HYBGEN", but hybgen_PPM_coefs has been replaced by edge_values_explicit_h4cw and PPM_monotonicity for flexibility and to simplify upgrades. Answers with existing INTERPOLATION_SCHEME options are unchanged.

REMAPPING_SCHEME="PPM_CW" is a new option which can perform better than "P1M_H2" when used with INTERPOLATION_SCHEME="PPM_CW". Answers with existing REMAPPING_SCHEME options are unchanged.

HYCOM1 regridding walks a monotonic density profile to locate the new interface locations where the interface density equals the target density. However, it assumes that moving one interface has no effect on the density at all other interfaces and this need not be the case. When regridding, with HYCOM1_ONLY_IMPROVES=True, an interface is only moved if this improves the fit to its target density. The default of False does not change answers.

Add "PPM_CW" as an option for INTERPOLATION_SCHEME and REMAPPING_SCHEME.
This implements the original Colella and Woodward (1984) edge calculation
for PPM. It computes 4th order explicit edge values but constrains
them to produce a monotonic profile, which is particularly effective
for remapping.

INTERPOLATION_SCHEME="PPM_CW" is identical to "REMAPPING_PPM_HYBGEN",
but hybgen_PPM_coefs has been replaced by edge_values_explicit_h4cw
and PPM_monotonicity for flexibility and to simplify upgrades.
Answers with existing INTERPOLATION_SCHEME options are unchanged.

REMAPPING_SCHEME="PPM_CW" is a new option which can perform better
than "P1M_H2" when used with INTERPOLATION_SCHEME="PPM_CW".  Answers
with existing REMAPPING_SCHEME options are unchanged.

HYCOM1 regridding walks a monotonic density profile to locate the
new interface locations where the interface density equals the target
density.  However, it assumes that moving one interface has no effect
on the density at all other interfaces and this need not be the case.
When regridding, with HYCOM1_ONLY_IMPROVES=True, an interface is only
moved if this improves the fit to its target density.  The default of
False does not change answers.
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #341 (061284c) into dev/gfdl (37389b5) will decrease coverage by 0.07%.
The diff coverage is 0.75%.

❗ Current head 061284c differs from pull request most recent head 3fdc31e. Consider uploading reports for the commit 3fdc31e to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #341      +/-   ##
============================================
- Coverage     37.15%   37.09%   -0.07%     
============================================
  Files           265      265              
  Lines         74516    74638     +122     
  Branches      13839    13851      +12     
============================================
- Hits          27689    27687       -2     
- Misses        41733    41856     +123     
- Partials       5094     5095       +1     
Impacted Files Coverage Δ
src/ALE/MOM_ALE.F90 45.09% <ø> (ø)
src/ALE/MOM_regridding.F90 27.00% <0.00%> (-0.09%) ⬇️
src/ALE/PPM_functions.F90 77.77% <0.00%> (-9.73%) ⬇️
src/ALE/coord_hycom.F90 0.00% <0.00%> (ø)
src/ALE/regrid_edge_values.F90 22.20% <0.00%> (-1.89%) ⬇️
src/ALE/regrid_interp.F90 1.49% <0.00%> (-0.11%) ⬇️
src/ALE/MOM_remapping.F90 73.61% <11.11%> (-0.77%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

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 contribution, @awallcraft. After carefully reading through these proposed changes, they all make sense to me, and I agree with them. This PR should be merged in after it has passed the pipeline regression testing and comes to the top of the queue of PRs.

(Unfortunately, the computer we use for this testing has been in bad shape for the past few weeks, and is completely offline this week, so it may be a while before we area able to run this through the standard tests and merge it into dev/gfdl.)

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/18680 ✔️

@marshallward marshallward merged commit 588cf03 into NOAA-GFDL:dev/gfdl Mar 30, 2023
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.

3 participants