Skip to content

Scale tideamp to L/T instead of Z/T#1087

Closed
breichl wants to merge 3 commits intoNOAA-GFDL:dev/gfdlfrom
breichl:tideamp_scaling_bug
Closed

Scale tideamp to L/T instead of Z/T#1087
breichl wants to merge 3 commits intoNOAA-GFDL:dev/gfdlfrom
breichl:tideamp_scaling_bug

Conversation

@breichl
Copy link
Copy Markdown

@breichl breichl commented Apr 15, 2026

  • Setting BBL_USE_TIDAL_BG = True causes the model to fail dimensional consistency test. If you set the spatial scale as m_to_L instead of m_to_Z in MOM_set_viscosity it fixes the issue.

@breichl breichl requested a review from Hallberg-NOAA April 15, 2026 18:49
@breichl breichl added the bug Something isn't working label Apr 15, 2026
@Hallberg-NOAA
Copy link
Copy Markdown
Member

I have examined the code related to this change, and I agree with it, but the comment describing the units of CS%tideamp on line 136 of MOM_set_viscosity.F90 also need to be corrected, from [Z T-1 ~> m s-1] to [L T-1 ~> m s-1].

@breichl
Copy link
Copy Markdown
Author

breichl commented Apr 16, 2026

Got it. Thanks for looking at this and catching that, @Hallberg-NOAA!

brandon.reichl added 2 commits April 17, 2026 14:25
- Setting BBL_USE_TIDAL_BG = True causes the model to fail dimensional consistency test.  If you set the spatial scale as m_to_L instead of m_to_Z in MOM_set_viscosity it fixes the issue.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a dimensional-scaling inconsistency when BBL_USE_TIDAL_BG = True by ensuring tidal amplitude input is scaled as a horizontal velocity (L/T) rather than a vertical velocity (Z/T), resolving the dimensional consistency test failure described in the PR.

Changes:

  • Update tideamp units annotation to [L T-1] to reflect a horizontal velocity.
  • Rescale tideamp input read from US%m_to_Z*US%T_to_s to US%m_to_L*US%T_to_s in set_visc_init.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/parameterizations/vertical/MOM_set_viscosity.F90 Outdated
Comment thread src/parameterizations/vertical/MOM_set_viscosity.F90 Outdated
Change suggested Hallberg

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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.

Tested and passed at https://gitlab.gfdl.noaa.gov/ogrp/mom6ci/MOM6/-/pipelines/30583.

Handling merge on the command line so we can avoid a modified commit message

@adcroft
Copy link
Copy Markdown
Member

adcroft commented Apr 17, 2026

Thie PR was merged manually in c925265

@adcroft adcroft closed this Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants