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

Fix rescaling of internal tide debugging code #763

Open
wants to merge 1 commit into
base: dev/gfdl
Choose a base branch
from

Conversation

Hallberg-NOAA
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA commented Nov 30, 2024

Corrected the internal tide rescaling arguments so that some of the debugging variables have units that are consistent with their documented units. This involved changing the scale arguments to global_area_integral() to tmp_scale arguments in 8 places so that the units of the output retain the scaling of the input variable. The multiplication by the reverse of the scaling factor was also added to 4 debugging output messages.

The documented units of internal wave energies in 5 register_restart_field calls were previously given as "m3 s-2" in non-Boussinesq mode and "J m-2" in Boussinesq mode when the reverse was actually true. This has now been corrected.

Also replaced the scale argument to 35 chksum() calls with the equivalent but preferred unscale argument, following the pattern elsewhere in the MOM6 code.

All answers are bitwise identical, and only debugging code was modified.

@awallcraft
Copy link

Pull request #764 notes that MOM_internal_tides.F90 may be defining or calculating frequency incorrectly. So far as I can tell this does not impact the debugging code changed here, but any test cases used to confirm the diagnostics may be able to resolve the frequency issue.

@Hallberg-NOAA
Copy link
Member Author

Hallberg-NOAA commented Nov 30, 2024

The code itself was not incorrect, per se, but it was inconsistent with the descriptions of the units of the debugging diagnostic variables in the comments describing them. The revised code will give equivalent model output, but it is now internally self-consistent with how it is described. I agree that this PR should be essentially independent of the corrections being made in PR #764.

@Hallberg-NOAA Hallberg-NOAA changed the title Fix rescaling of internal tide of debugging code Fix rescaling of internal tide debugging code Dec 9, 2024
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.

You asked for @raphaeldussin to review, but IMHO the changes outside of internal tides look good to go.

@Hallberg-NOAA Hallberg-NOAA added documentation Improvements or additions to documentation refactor Code cleanup with no changes in functionality or results labels Dec 12, 2024
@Hallberg-NOAA
Copy link
Member Author

This rescaling of the debugging options will work as intended and documented in the comments, provided that PR #778 is handled first.

Copy link

@raphaeldussin raphaeldussin left a comment

Choose a reason for hiding this comment

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

debugging prints with different rescaling powers show identical results

  Corrected the internal tide rescaling arguments so that some of the debugging
variables have units that are consistent with their documented units.  This
involved changing the scale arguments to global_area_integral to tmp_scale
arguments in 8 places so that the units of the output retain the scaling of the
input variable.  The multiplication by the reverse of the scaling factor was
also added to 4 debugging output messages.

  The documented units of internal wave energies in 5 register_restart_field
calls were previously given as "m3 s-2" in non-Boussinesq mode and "J m-2" in
Boussinesq mode when the reverse was actually true.  This has now been
corrected.

  Also replaced the scale argument to 35 chksum calls with the equivalent but
preferred unscale argument, following the pattern elsewhere in the MOM6 code.

  All answers are bitwise identical, and only debugging code was modified.
@Hallberg-NOAA Hallberg-NOAA force-pushed the better_int_tide_rescaling branch from 6fd0efb to 7f3cada Compare December 29, 2024 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation refactor Code cleanup with no changes in functionality or results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants