wave structure computation into wave_speeds

[raphaeldussin]

wave_speeds now computes the wave structures
(eigenvectors) for each mode speed (eigenvalue)
similarly to the wave_speed (singular) function.
This is a replacement for the MOM_wave_structure
function, which could be removed in a subsequent PR.

Additional arrays for mode structures and integral
quantities are passed as output hence this is a
breaking change for the call to wave_speeds.
However it is only called once in diabatic_driver
and is used exclusively for internal tides ray tracing.
This also requires to make int_tides structure public,
this could be reverted by moving wave_speeds into
MOM_internal_tides if necessary in a following PR.

The dimensional solutions for the wave structures are
now computed inside MOM_internal_tides, and new
diagnostics are added.

An out-of-bounds bug is also corrected for the computation
of an averaged Coriolis parameter.

[codecov]

Codecov Report

Merging #354 (7b7e76c) into dev/gfdl (e86b35a) will increase coverage by 0.08%.
The diff coverage is 0.00%.

❗ Current head 7b7e76c differs from pull request most recent head 9b616c7. Consider uploading reports for the commit 9b616c7 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #354      +/-   ##
============================================
+ Coverage     38.29%   38.38%   +0.08%     
============================================
  Files           269      268       -1     
  Lines         76180    76010     -170     
  Branches      14011    13987      -24     
============================================
+ Hits          29174    29176       +2     
+ Misses        41767    41595     -172     
  Partials       5239     5239              

Impacted Files	Coverage Δ
src/diagnostics/MOM_wave_speed.F90	37.34% <0.00%> (-6.84%)	⬇️
...c/parameterizations/lateral/MOM_internal_tides.F90	0.00% <0.00%> (ø)
...parameterizations/vertical/MOM_diabatic_driver.F90	46.06% <0.00%> (ø)

... and 1 file with indirect coverage changes

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

[Hallberg-NOAA]

Thank you for this PR. For the most part, this looks like it should work, but there is something about this PR has me confused. Why was it necessary to move the subroutine wave_speeds() from MOM_wave_speed to MOM_internal_tides? Unless there is a compelling reason for this move, I think that it should be returned to the MOM_wave_speed module.

This structure is problematic because wave_speeds() is still using the same wave_speed_CS control structure from the MOM_wave_speed module, which is now being made transparent. It is also problematic because it makes it hard to see what changes needed to be made in wave_speeds() to make it work properly.

This PR passes our testing, but this could be mostly because the impacted code is not being exercised by our testing. The visual inspection of these changes is therefore all the more important. If the move between modules is in fact necessary for some compelling reason, please consider making these changes in two separate PRs, one of which makes the necessary code changes, and the other does nothing other than move the wave_speeds subroutine.

[raphaeldussin]

The move of wave_speeds into the MOM_internal_tides module was suggested in dev meeting so that we don't need to expose the internal_tides_CS in MOM_diabatic_driver during the call to wave_speeds, the later has been moved out of MOM_diabatic_driver and into propagate_int_tide also to keep int_tides_CS private. wave_speeds indeed uses a now public wave_speed_CS but only reading in parameters, no arrays are passed through this structure. This seemed like a safe solution that avoid duplicating the control structure. Regarding the refactoring, this PR can be decomposed into 3 steps:

commit 1: changes to the wave_speeds code, this is the only answer changing part
commit 2: removed the now unused wave_structure code
commit 3: refactoring: move wave_speeds into MOM_internal_tides

I can split into 3 different PRs if that's easier to review, with the drawback of adding to the regression pipeline.

[raphaeldussin]

I've undone commit 3 which was conflicting with your rebase @Hallberg-NOAA

[Hallberg-NOAA]

These changes have addressed the primary issues with the previous version. There should be some future refactoring which will avoid the need for transparent types, but holding this up any longer would be disruptive of important development efforts.

[marshallward]

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