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 internal wave refraction #1458

Merged
merged 7 commits into from
Aug 17, 2021

Conversation

raphaeldussin
Copy link
Contributor

current code uses halo (or land) points in the computation of the gradient of Internal Wave group velocity, which results in spurious gradients and unexpected refraction close to boundaries. This PR computes IW group velocities on U and V points, discarding contribution from halo or land, before computing the gradient.

@codecov
Copy link

codecov bot commented Aug 9, 2021

Codecov Report

Merging #1458 (bcbf43c) into dev/gfdl (e88cdaa) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl    #1458      +/-   ##
============================================
- Coverage     29.13%   29.12%   -0.01%     
============================================
  Files           235      235              
  Lines         71038    71045       +7     
============================================
  Hits          20694    20694              
- Misses        50344    50351       +7     
Impacted Files Coverage Δ
...c/parameterizations/lateral/MOM_internal_tides.F90 0.00% <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 e88cdaa...bcbf43c. Read the comment docs.

@raphaeldussin raphaeldussin marked this pull request as draft August 9, 2021 16:43
@marshallward
Copy link
Collaborator

After a chat with @raphaeldussin we came up with the following recommended changes:

  • Loops at L799 and L811 should swap the i and j loops, so that i is innermost

  • The c / (wgt1 + wgt2) should probably replaced with a precomputed mask containing 0., 1., or 0.5.
    Two benefits:

    • Eliminate the floating point division (since it's unlikely a compiler will
    • Remove the if-blocks with some sort of mask

    But at the moment I don't have a good suggestion on how to efficiently construct this mask.

We will sit on this one for a bit while Raf looks into some other issues, and will settle on a solution when that other work is finished.

@raphaeldussin raphaeldussin marked this pull request as ready for review August 16, 2021 19:39
@marshallward
Copy link
Collaborator

The method by @raphaeldussin for computing cn_[uv] looks good to me 👍

@marshallward
Copy link
Collaborator

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

@marshallward marshallward merged commit a29bff9 into mom-ocean:dev/gfdl Aug 17, 2021
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