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

(*)Simplify interpolate_column weight calculations #227

Merged

Conversation

Hallberg-NOAA
Copy link
Member

Set the interpolation weights inside of interpolate_column to explicitly be the complement of one another, thereby saving an extra division at each point and reducing the number of variables that need to be stored, in preparation for the creation of a separate subroutine to find interface positions. This commit is mathematically equivalent to what was there before, and the extensive unit testing of interpolate_column is still passing, but it changes the value of some interpolated interface diagnostics at the level of roundoff (but not the MOM6 solutions themselves, as they do not depend on interpolate_column yet).

Some of the C.I. tests for regressions of diagnostics are expected to fail to reproduce the previous values, but they do agree to many (13 or more) decimal places, apart from near-zero spatial means of these diagnostics (where the mean is many orders of magnitude smaller than the values that are being averaged) that can differ at leading order. All of the differences are well within the range of what would be expected from changes at roundoff in the interpolated interface diagnostics themselves.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #227 (c0af21e) into dev/gfdl (e26d8da) will increase coverage by 1.29%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##           dev/gfdl     #227      +/-   ##
============================================
+ Coverage     35.93%   37.23%   +1.29%     
============================================
  Files           260      263       +3     
  Lines         72002    73060    +1058     
  Branches      13590    13605      +15     
============================================
+ Hits          25872    27201    +1329     
+ Misses        41072    40841     -231     
+ Partials       5058     5018      -40     
Impacted Files Coverage Δ
src/ALE/MOM_remapping.F90 74.51% <75.00%> (ø)
src/framework/MOM_unit_testing.F90 74.66% <0.00%> (ø)
...ig_src/drivers/unit_tests/MOM_unit_test_driver.F90 92.00% <0.00%> (ø)
src/framework/testing/MOM_file_parser_tests.F90 94.15% <0.00%> (ø)
src/framework/MOM_string_functions.F90 86.82% <0.00%> (+0.97%) ⬆️
config_src/infra/FMS1/MOM_coms_infra.F90 62.32% <0.00%> (+2.73%) ⬆️
config_src/infra/FMS1/MOM_error_infra.F90 100.00% <0.00%> (+7.69%) ⬆️
src/framework/MOM_document.F90 72.76% <0.00%> (+11.83%) ⬆️
src/framework/MOM_error_handler.F90 59.77% <0.00%> (+20.68%) ⬆️
src/framework/MOM_file_parser.F90 90.78% <0.00%> (+30.58%) ⬆️
... and 1 more

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

  Set the interpolation weights inside of interpolate_column to explicitly be
the complement of one another, thereby saving an extra division at each point
and reducing the number of variables that need to be stored, in preparation for
the creation of a separate subroutine to find interface positions.  This commit
is mathematically equivalent to what was there before, and the extensive unit
testing of interpolate_column is still passing, but it changes the value of some
interpolated interface diagnostics at the level of roundoff (but not the MOM6
solutions themselves, as they do not depend on interpolate_column yet).
@Hallberg-NOAA Hallberg-NOAA force-pushed the interpolate_column_reweight branch from 67d9625 to c0af21e Compare October 31, 2022 21:57
Copy link
Member

@adcroft adcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the PR description, this changes the output of variables on interfaces but does not change the solution. Testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17322 . Assuming this passes regressions, I approve this change.

@Hallberg-NOAA
Copy link
Member Author

The testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17322 has passed the regressions.

@marshallward
Copy link
Member

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

@marshallward marshallward merged commit 82cbb09 into NOAA-GFDL:dev/gfdl Nov 7, 2022
@Hallberg-NOAA Hallberg-NOAA deleted the interpolate_column_reweight branch February 2, 2023 13:28
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