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 new scaling bug in ML restrat minimum ustar #265

Merged

Conversation

Hallberg-NOAA
Copy link
Member

There are extra US%T_to_s scaling factors in the expressions for ustar_min that were recently introduced with dev/gfdl PR #251; these are duplicative of the scaling factor that is already being applied when the parameter OMEGA is read in. The resulting expressions for ustar_min therefore effectively have units of [Z s T-2 ~> m s-1] when they should have units of [Z T-1 ~> m s-1]. Because ustar_min is a tiny floor on the magnitude of ustar, there is a range of values of T_RESCALE_POWER that will give the same answers as when it is 0, but for large enough values the answers will change, perhaps dramatically. This small commit removes these extra factors. Answers will change for some large values of T_RESCALE_POWER, but they are bitwise identical in the TC testing and in MOM6-examples based regression tests with modest or negative values.

@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working answer-changing A change in results (actual or potential) labels Dec 4, 2022
@codecov
Copy link

codecov bot commented Dec 4, 2022

Codecov Report

Merging #265 (8b6b273) into dev/gfdl (5ef380c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           dev/gfdl     #265      +/-   ##
============================================
- Coverage     37.13%   37.13%   -0.01%     
============================================
  Files           263      263              
  Lines         73444    73444              
  Branches      13674    13674              
============================================
- Hits          27272    27270       -2     
- Misses        41147    41149       +2     
  Partials       5025     5025              
Impacted Files Coverage Δ
...ameterizations/lateral/MOM_mixed_layer_restrat.F90 73.64% <100.00%> (ø)
src/framework/MOM_document.F90 73.88% <0.00%> (-0.45%) ⬇️

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

  There are extra US%T_to_s scaling factors in the expressions for ustar_min
that were recently introduced with dev/gfdl PR mom-ocean#251; these are duplicative of
the scaling factor that is already being applied when the parameter OMEGA is
read in.  The resulting expressions for ustar_min therefore effectively have
units of [Z s T-2 ~> m s-1] when they should have units of [Z T-1 ~> m s-1].
Because ustar_min is a tiny floor on the magnitude of ustar, there is a range of
values of T_RESCALE_POWER that will give the same answers as when it is 0, but
for large enough values the answers will change, perhaps dramatically.  This
small commit removes these extra factors.  Answers will change for some large
values of T_RESCALE_POWER, but they are bitwise identical in the TC testing and
in MOM6-examples based regression tests with modest or negative values.
@Hallberg-NOAA Hallberg-NOAA force-pushed the ML_restrat_ustar_min_bugfix branch from deb79f6 to 8b6b273 Compare December 16, 2022 13:15
@marshallward
Copy link
Member

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

@marshallward marshallward merged commit 7173432 into NOAA-GFDL:dev/gfdl Dec 18, 2022
@Hallberg-NOAA Hallberg-NOAA deleted the ML_restrat_ustar_min_bugfix branch February 2, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answer-changing A change in results (actual or potential) bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants