-
Notifications
You must be signed in to change notification settings - Fork 195
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
Tripolar grid in Oceananigans #3913
base: main
Are you sure you want to change the base?
Conversation
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.
nice
we should update
Oceananigans.jl/docs/src/grids.md
Lines 86 to 93 in dc4a327
1. [`RectilinearGrid`](@ref Oceananigans.Grids.RectilinearGrid) can be fashioned into lines, rectangles and boxes. | |
2. [`LatitudeLongitudeGrid`](@ref Oceananigans.Grids.LatitudeLongitudeGrid) represents sectors of thin spherical shells, with cells bounded by lines of constant latitude and longitude. | |
3. [`OrthogonalSphericalShellGrid`](@ref Oceananigans.Grids.OrthogonalSphericalShellGrid) represents sectors of thin spherical shells divided with mesh lines that intersect at right angles (thus, orthogonal) but are otherwise arbitrary. | |
!!! note "OrthogonalSphericalShellGrids.jl" | |
See the auxiliary package [`OrthogonalSphericalShellGrids.jl`](https://github.com/CliMA/OrthogonalSphericalShellGrids.jl) | |
for recipes that implement some useful `OrthogonalSphericalShellGrid`, including the | |
["tripolar" grid](https://www.sciencedirect.com/science/article/abs/pii/S0021999196901369). |
…ceananigans.jl into ss/tripolar-grid-in-oceananigans
Which Oceananigans.jl/src/Grids/tripolar_grid.jl Line 108 in db49f4d
WARNING: both Architectures and CUDA export "device"; uses of it in module Grids must be qualified
Unit tests...: Error During Test at /Users/navid/Research/OC11.jl/test/test_tripolar_grid.jl:31
Got exception outside of a @test
UndefVarError: `device` not defined
Stacktrace:
[1] (TripolarGrid)(arch::CPU, FT::DataType; size::Tuple{Int64, Int64, Int64}, southernmost_latitude::Int64, halo::Tuple{Int64, Int64, Int64}, radius::Float64, z::Tuple{Int64, Int64}, north_poles_latitude::Int64, first_pole_longitude::Int64)
@ Oceananigans.Grids ~/Research/OC11.jl/src/Grids/tripolar_grid.jl:108 |
Why draft? |
I guess because it needs work... Still most test fail. It's not ready for review... |
Huh true. Just thought it would have been a copy/paste job but maybe there's more to it |
It shouldn't be hard to fix... I can give it a go later! |
the main problem is that we use Fields and |
This part: Oceananigans.jl/src/Grids/tripolar_grid.jl Lines 139 to 148 in 44f62c1
requires BoundaryConditions before grids? |
Oh I just saw your comment @simone-silvestri! |
Damn... Fields and |
We could move the zipper-filling halo kernels in grids and use them just as kernels. |
Or we can define the TripolarGrid in a module on its own after Fields/Boundary conditions? It does make sense that it lives in Grids... aaaaargh |
Btw, here we define a haversine Oceananigans.jl/src/Grids/tripolar_grid.jl Lines 457 to 467 in 44f62c1
Elsewhere in Oceananigans we use the Distances.haversine; see https://github.com/JuliaStats/Distances.jl/blob/master/src/haversine.jl |
The two are exactly the same; the snippet below demonstrates that: using Oceananigans.Grids: lat_lon_to_cartesian
using Distances
# Is this the same as in Oceananigans?
# TODO: check it out
function haversine_simone(a, b, radius)
λ₁, φ₁ = a
λ₂, φ₂ = b
x₁, y₁, z₁ = lat_lon_to_cartesian(φ₁, λ₁, radius)
x₂, y₂, z₂ = lat_lon_to_cartesian(φ₂, λ₂, radius)
return radius * acos(max(-1.0, min((x₁ * x₂ + y₁ * y₂ + z₁ * z₂) / radius^2, 1.0)))
end
using Test
for i in 1:100
(λ1, φ1) = rand()*360, (2rand()-1)*90
(λ2, φ2) = rand()*360, (2rand()-1)*90
radius = rand() * 6300e3
@test haversine_simone((λ1, φ1), (λ2, φ2), radius) ≈ Distances.haversine((λ1, φ1), (λ2, φ2), radius)
end |
@simone, what's the paper mentioned here:
|
The date was wrong 😅 |
…ceananigans.jl into ss/tripolar-grid-in-oceananigans
Let's see if it works with the current workaround |
Should we change how the code is organized? Can we put We could also have a module |
Hmmm, I think Grids should be before Fields. Oceananigans.jl/src/Grids/tripolar_grid.jl Lines 234 to 260 in ce8c49e
and then use it in boundary conditions: Oceananigans.jl/src/BoundaryConditions/fill_halo_regions_zipper.jl Lines 47 to 56 in ce8c49e
|
what about having an It makes sense to me that for non-trivial grids, we actually want to use grids and fields to build the grid. For example, the cubed sphere grid is a conformal map of a rectilinear grid. Just a thought to save some work but if you think it will ultimate accelerate progress (for some reason) to put everything in |
Well, I guess if we move all the tripolar grid construction in a I am ok with this solution if you guys think we can do this at this moment. This would simplify all the construction of non-trivial grids in the future (probably the tripolar grid will not be the last). We could reassess this implementation in the future maybe? |
I think that's a nice plan for sure. It's nice to have the basic utilities available when building new grids via conformal maps, etc. We can also implement the rotated lat-lon grid there? |
…ceananigans.jl into ss/tripolar-grid-in-oceananigans
I think it'll be cleaner if indeed we have OrthogonalSphericalShellGrids as a module after Fields (and BoundaryConditions? -- seems like the tripolar needs that as well?) |
Our implementation of the tripolar grid is in https://github.com/CliMA/OrthogonalSphericalShellGrids.jl at the moment.
We have figured out that the implementation of the tripolar grid is very much entangled with Oceananigans because it requires extending a lot of time-stepping specific code.
This leads to delays in development when we have to develop the two packages in tandem.
Therefore, this PR migrates the implementation of the Tripolar grid and its specific methods to Oceananigans.