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

Diagnostic scaling fixes #991

Merged
merged 11 commits into from
Sep 6, 2019

Conversation

marshallward
Copy link
Collaborator

This patch fixes an issue in the time scalings of the tendencies in MOM_diagnostics, where the timestep (dt) was not being scaled, and was only corrected in some of the variables.

This patch also passes through the scaling of the KE and the layer KE tendency diagnostic conversions.

This patch adds and fixes the scaling factors for several diagnostics.
This patch fixes an issue in the time scalings of the tendencies in
MOM_diagnostics, where the timestep (dt) was not being scaled, and was
only corrected in some of the variables.

This patch also passes through the scaling of KE and dKE_dt onto the
diagnostic conversion factors.
Finished the rescaling of the layer KE tendency diagnostics, which are
all now passing the diagnostic checksum scaling tests.
@codecov-io
Copy link

codecov-io commented Sep 4, 2019

Codecov Report

Merging #991 into dev/gfdl will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl     #991      +/-   ##
============================================
- Coverage     43.24%   43.24%   -0.01%     
============================================
  Files           213      213              
  Lines         62214    62214              
============================================
- Hits          26905    26903       -2     
- Misses        35309    35311       +2
Impacted Files Coverage Δ
src/core/MOM.F90 69.2% <ø> (ø) ⬆️
src/tracer/MOM_tracer_hor_diff.F90 87.21% <100%> (ø) ⬆️
src/tracer/MOM_tracer_registry.F90 89.84% <100%> (ø) ⬆️
src/core/MOM_barotropic.F90 71.92% <100%> (ø) ⬆️
src/diagnostics/MOM_diagnostics.F90 87.38% <100%> (ø) ⬆️
...arameterizations/vertical/MOM_bulk_mixed_layer.F90 57.21% <100%> (ø) ⬆️
src/framework/MOM_spatial_means.F90 26.14% <100%> (ø) ⬆️
...c/parameterizations/vertical/MOM_vert_friction.F90 72.12% <100%> (ø) ⬆️
...rc/parameterizations/vertical/MOM_tidal_mixing.F90 9.87% <0%> (-0.32%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f790eb...d64b472. Read the comment docs.

This patch fixes the scaling of the diagnostic parameters in the [uv]hbt
and [uv]hbt_hifreq transport diagnostics.
@marshallward
Copy link
Collaborator Author

Might as well keep this PR open to gather up other incoming scaling fixes.

@marshallward marshallward changed the title Tendency scaling Diagnostic scaling fixes Sep 4, 2019
Length scaling for global averaging is no longer required and has been
removed.  This has fixed the dimensional scaling of any diagnostics
using this function.
Diagnostics of diffusive tracer fluxes (*_diff[xy], *_diff[xy]_2d) were
not scaling due to issues with the Coef_x term, which was in units of
L2 while the tracer itself was in units of m2.

Although the Coef_x term was being de-scaled to meters, the cumulative
sum was failing to reproduce for many tracers, either being reverted (in
temp) or failing to reproduce all bits (in salt).

We resolve this by re-defining the diffusive flux terms as H L2 s-1,
rather than H m2 s-1, and moving the scaling to the final conversion
factor, which restored reproducibility of all tracers.

This patch contains an API change.  We have added the unit scaling
struct (US) to the register_tracer_diagnostic function.
Dimensional scaling factor was added to the MEKE_src diagnostic, fixing
the rescaling reproducibility.
This patch fixes a bug in the registration of CAu, where the conversion
argument had been included in the string of the dimensions of the
diagnostic registration.

The units of various accelerations have also been changed from 'meter
second-2' to 'm s-2' for consistent with the rest of the model.
This patch fixes the dimensional scaling of the pre-ALE u and v
diagnostics, and the uhml and vhml mixed layer diagnostics.
@marshallward
Copy link
Collaborator Author

Most of the low hanging fruit related to dimensional scaling has been fixed in this PR, so this should be OK to merge after review.

Gaea regression testing: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/8857

Copy link
Collaborator

@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, @marshallward, for these corrections to the dimensional rescaling of the diagnostic output. The fact that we can now systematically catch such missing or incorrect scaling factors for MOM6 diagnostics is a significant advance for the MOM6 code.

This PR should be merged in once the corresponding MOM6-examples pipeline test has passed.

@marshallward
Copy link
Collaborator Author

Regression tests have passed, so this is ready to merge.

This doesn't address all of the scaling problems, but has resolved the majority of them.

@Hallberg-NOAA Hallberg-NOAA merged commit eb56c68 into mom-ocean:dev/gfdl Sep 6, 2019
@marshallward marshallward deleted the tendency_scaling branch February 13, 2020 16:39
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