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

(*)Use conversion factor for masscello diagnostic #335

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

Hallberg-NOAA
Copy link
Member

Use a conversion factor to rescale the units of masscello, just like every other diagnostic. This does not change the diagnostic itself, but it changes the order of the rescaling and the vertical remapping of this diagnostic onto other coordinates (like z) or spatial averaging of this diagnostic, which can change values in the last bits for this diagnostic for Boussinesq models (but not for non-Boussinesq models, for which the conversion factor is an integer power of 2). As a result some of the diagnostics derived from masscello can differ and this commit nominally fails the TC testing for reproducibility across code versions. All solutions and primary diagnostics, however, are bitwise identical, and even the derived diagnostic calculations are mathematically equivalent.

  Use a conversion factor to rescale the units of masscello, just like every
other diagnostic.  This does not change the diagnostic itself, but it changes
the order of the rescaling and the vertical remapping of this diagnostic onto
other coordinates (like z) or spatial averaging of this diagnostic, which can
change values in the last bits for this diagnostic for Boussinesq models (but
not for non-Boussinesq models, for which the conversion factor is an integer
power of 2).  As a result some of the diagnostics derived from masscello can
differ and this commit nominally fails the TC testing for reproducibility across
code versions.  All solutions and primary diagnostics, however, are bitwise
identical, and even the derived diagnostic calculations are mathematically
equivalent.
@Hallberg-NOAA
Copy link
Member Author

This commit is expected to fail the TC tests for exact diagnostic consistency across model versions. As a result, this should not be merged into dev/gfdl until the pending voluminous PR from dev/gfdl to MOM-ocean/main has been created.

@codecov
Copy link

codecov bot commented Mar 12, 2023

Codecov Report

Merging #335 (b1e1921) into dev/gfdl (4038d69) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head b1e1921 differs from pull request most recent head 6c48830. Consider uploading reports for the commit 6c48830 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #335      +/-   ##
============================================
- Coverage     37.07%   37.07%   -0.01%     
============================================
  Files           264      264              
  Lines         74353    74351       -2     
  Branches      13788    13788              
============================================
- Hits          27569    27563       -6     
- Misses        41686    41690       +4     
  Partials       5098     5098              
Impacted Files Coverage Δ
src/diagnostics/MOM_diagnostics.F90 71.52% <100.00%> (-0.07%) ⬇️

... 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

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

Regression makes sense here.

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/18867 ✔️
No regressions in our suite.

@marshallward marshallward merged commit 6547b2a into NOAA-GFDL:dev/gfdl Apr 21, 2023
@Hallberg-NOAA Hallberg-NOAA deleted the masscello_conversion branch May 10, 2024 22:12
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