-
Notifications
You must be signed in to change notification settings - Fork 65
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
Rotate vels #614
Rotate vels #614
Conversation
- This is required for our BOEM-funded Arctic project. It needs something similar for SIS2 as well.
- OK to squash merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me, but I do have one change that I would like to request. In the upcoming set of revisions to give rotational symmetry with Fused-Multiply-Adds (FMAs), we are going to be writing expressions like Ax * Bx + Ay * By
as (Ax * Bx) + (Ay * By)
so that the compiler does not have the option of fusing the x- or y- parts of the expressions but not the other one. It would be helpful if you could revise the expressions for ssu_east
and ssv_north
to either add the parentheses to prevent FMAs from adding an asymmetry, or at least write these expressions so that the cos_rot
terms come first in both cases. This is, of course, a diagnostic so perhaps we need not be so particular here, but for a new contribution like this, it is probably easier to make this small change now rather than coming back to it later.
There was a problem hiding this 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 updated PR.
@kshedstrom Can you enable "Allow rebase merging" in the Settings of ESMG/MOM6? This would allow us to update this PR to the latest dev/gfdl. |
It looks like it is already set. Is it not behaving that way? |
I'm not able to update this PR. But let's try to sort this out privately. |
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/23445 ✔️ 🟡 This requires an update to |
This PR was rebased into this commit: bb8a653 |
This adds an option to save true north ssu and ssv, required for our BOEM contract. There are similar changes to SIS2.