-
Notifications
You must be signed in to change notification settings - Fork 190
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
Bug in the vector rotation operator #3744
Conversation
We need more tests |
Can we add a test that uses |
test/test_cubed_spheres.jl
Outdated
u = XFaceField(grid) | ||
v = YFaceField(grid) | ||
|
||
# Purely zonal flow. |
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.
not for all grids right?
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.
right, it's in the extrinsic frame. I have changed the comment
Perhaps we don't have this, but can we rotate a |
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.
I see that the challenge of writing tests is that we don't have a good way of generating general grids...
@test all(sic_model.velocities.v.data[i, j, :] .≈ per_model[i, j].velocities.v.data[1, 1, :]) | ||
@test all(sic_model.tracers.c.data[i, j, :] .≈ per_model[i, j].tracers.c.data[1, 1, :]) | ||
end | ||
|
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.
was this sent to the wrong PR?
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.
yeah this was meant to be in #3741. I have moved it there
The meridional velocity had the wrong sign in the vector rotation operator for switching from extrinsic to intrinsic coordinates in an
OrthogonalSphericalShellGrid
.The test was also poorly designed (my bad) and conspired to hide the bug. This PR should fix the rotation and update the test to make sure everything is correct.