Skip to content

extension to the internal tides module#481

Merged
marshallward merged 5 commits into
NOAA-GFDL:dev/gfdlfrom
raphaeldussin:internal_tides_split_freqs_modes
Oct 28, 2023
Merged

extension to the internal tides module#481
marshallward merged 5 commits into
NOAA-GFDL:dev/gfdlfrom
raphaeldussin:internal_tides_split_freqs_modes

Conversation

@raphaeldussin
Copy link
Copy Markdown

the module in now able to read in tidal velocities for different tidal harmonics and distribute the energy and distribute TKE input over the different vertical modes. This involves upsizing dimensions of several arrays and mofiying some API. internal_tide_input_CS is promoted to public to facilitate the passing of energy input to MOM_internal_tides

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 15, 2023

Codecov Report

Merging #481 (cefe39f) into dev/gfdl (ddb88f8) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

❗ Current head cefe39f differs from pull request most recent head 8878535. Consider uploading reports for the commit 8878535 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #481      +/-   ##
============================================
- Coverage     37.54%   37.50%   -0.04%     
============================================
  Files           270      270              
  Lines         79030    79111      +81     
  Branches      14635    14636       +1     
============================================
- Hits          29671    29670       -1     
- Misses        43892    43975      +83     
+ Partials       5467     5466       -1     
Files Coverage Δ
...parameterizations/vertical/MOM_diabatic_driver.F90 46.86% <0.00%> (ø)
...meterizations/vertical/MOM_internal_tide_input.F90 0.00% <0.00%> (ø)
...c/parameterizations/lateral/MOM_internal_tides.F90 0.00% <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

the module in now able to read in tidal velocities for different
tidal harmonics and distribute the energy and distribute TKE input over
the different vertical modes. This involves upsizing dimensions of
several arrays and mofiying some API. internal_tide_input_CS is
promoted to public to facilitate the passing of energy input to
MOM_internal_tides
@raphaeldussin raphaeldussin force-pushed the internal_tides_split_freqs_modes branch from 3978254 to e40f516 Compare September 15, 2023 17:51
@raphaeldussin raphaeldussin marked this pull request as ready for review September 25, 2023 14:51
@marshallward marshallward self-assigned this Sep 25, 2023
Copy link
Copy Markdown
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution. I appreciate the direction that this PR is going, but there are two significant issues that I think need to be discussed.

  1. The magnitude of the velocity in the quadratic bottom drag term should include all of the velocities added in quadrature, not just a single tidal component, as is the case with the modified code.

  2. If at all possible, we should explore other code constructs that avoid making any of our control structures transparent. Opaque structures avoid most of the issues related to efficiency and readability that arise from passing around what are effectively common blocks.

Copy link
Copy Markdown
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

With these change, the concerns that I had with the previous version have been addressed. The only other change I would suggest is that the allocate call for vel_btTide should be moved outside of get_barotropic_tidal_vel() so that its pairing with the call to deallocate(vel_btTide) is more readily evident, and similarly the allocate call for TKE_tidal_input should be moved out of get_input_TKE().

@marshallward
Copy link
Copy Markdown
Member

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

Squash-merging.

@marshallward marshallward merged commit 615e57f into NOAA-GFDL:dev/gfdl Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants