Fix a rescaling bug with MEKE_EQUILIBRIUM_ALT

[Hallberg-NOAA]

Corrected a bug in a dimensional rescaling factor that will cause test cases
using MEKE with MEKE_EQUILIBRIUM_ALT = True to fail dimensional consistency
testing. However, answers are unchanged when no rescaling is used. This minor
bug has been in the code since the MEKE_EQUILIBRIUM_ALT was first introduced in
2019. All answers in the existing MOM6-examples test suite are bitwise
identical.

[codecov]

Codecov Report

Merging #133 (0b9e19b) into dev/gfdl (27bb8b8) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           dev/gfdl     #133   +/-   ##
=========================================
  Coverage     33.45%   33.45%           
=========================================
  Files           262      262           
  Lines         71385    71385           
  Branches      13323    13323           
=========================================
  Hits          23884    23884           
  Misses        43029    43029           
  Partials       4472     4472           

Impacted Files	Coverage Δ
src/parameterizations/lateral/MOM_MEKE.F90	50.34% <0.00%> (ø)

---

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 27bb8b8...0b9e19b. Read the comment docs.

[adcroft]

Testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/15663

[klindsay28]

Curiously, this was previously fixed in mom-ocean#1491
At some point, which I haven't been able to figure out when, it got re-broken.
It has been refixed on the NCAR fork with NCAR#209

[adcroft]

Passed: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/15663

[marshallward]

Curiously, this was previously fixed in mom-ocean#1491 At some point, which I haven't been able to figure out when, it got re-broken. It has been refixed on the NCAR fork with NCAR#209

The commit appears in the history: 9011801b617ef6c09a2e48f8685532886122d10f

And yet the file reports that the scaling factor has been unchanged for years: https://github.com/mom-ocean/MOM6/blame/main/src/parameterizations/lateral/MOM_MEKE.F90

All a bit worrying. Maybe I failed to merge maih to dev/gfdl? That would be monumentally dumb, but I do recall some drama around it.

---

git branch --contains 9011801b617ef6c09a2e48f8685532886122d10f also reports that the commit is in both dev/gfdl and main. But no indication that a changed was ever reverted. It seems more like it was never applied.

[marshallward]

I spot checked other diffs from this PR contribution (other dimensional MEKE fixes, CFC code, etc), and as far as I can tell everything else is in there. So odd...

[klindsay28]

A possibility is that the change was lost during by-hand conflict resolution during a merge. My mod changed US%Z_to_m*G%bathyT to US%Z_to_L*G%bathyT. On another branch, commit 8a0ae944d181ec6d109a029568f27eb23d55e46f changed this expression to US%Z_to_m*depth_tot. Merging these, wherever it occurred, would have led to conflict requiring resolution. Perhaps the latter version was selected during conflict resolution, instead of combining them to obtain US%Z_to_L*depth_tot. The reversion was flagged in the NCAR/CESM test suite, because we use MEKE_equilibrium_alt == .true., so I don't think the reversion occurred here.

I suspect it was during a main->dev/gfdl merge. Testing of that merge must have missed this omission, I suspect because it didn't cover MEKE_equilibrium_alt == .true.. Because of the combinatorial explosion of options, it's unrealistic to expect every combination of options to be tested, so this is not surprising. However, I think it does highlight the risk of modifying code not covered by one's test suite.

[marshallward]

That sounds plausible. I don't see any other missing changes, and a hand-merge would be the place for a small error to appear. I was more worried about a total loss of the content, but that doesn't seem to have happened.

I guess we'll just have to be more careful. (Or develop better unit tests!)

[adcroft]

I accidentally replied while still editing - deleted and re-sent. This issue was indeed caused by a bad conflict resolution.

The GH history for MOM_MEKE.F90, https://github.com/NOAA-GFDL/MOM6/commits/dev/gfdl/src/parameterizations/lateral/MOM_MEKE.F90, shows things chronologically.
This makes it seem that the correction on Aug 23 by @klindsay28,

MOM6/src/parameterizations/lateral/MOM_MEKE.F90

Line 699 in 9011801

MEKE%MEKE(i,j) = (CS%MEKE_GEOMETRIC_alpha * SN * US%Z_to_L*G%bathyT(i,j))**2 / cd2

, was undone in the very next commit on Aug 26,

MOM6/src/parameterizations/lateral/MOM_MEKE.F90

Line 712 in 6979c29

MEKE%MEKE(i,j) = (CS%MEKE_GEOMETRIC_alpha * SN * US%Z_to_m*depth_tot(i,j))**2 / cd2

.

It turns out none of the commits in a line between 9011801..6979c29 are not at fault

$ git log --format="format:%h %ad %s" --date=short 9011801..6979c29 src/parameterizations/lateral/MOM_MEKE.F90

6979c29 2021-08-26 +()Add option for MEKE to calculate total depth
8a0ae94 2021-08-17 +Pass depth_tot around within MOM_MEKE
c72d441 2021-08-15 ()Fix dimensionally inconsistent MEKE beta calcs

That's because this relevant line was being edited on several branches at once:

Above you can see that 8a0ae94 changed G%bathyT to depth_tot on the same line which has corrected the scaling that was already on the dev/gfdl from NCAR. When conflict resolving in the merge between the two branches, 72aabe5#diff-2bf6c52474ad3142c4902b0f94ed67365c19e0bc7a6a61c4dcbc4e453330d258R712, neither option was correct without editing: either the depth_tot version needed the correct scaling applied or the correct scaling needed the change in variable. I think one was selected and the critical single character was missed.