-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add internal tide example in Docs #3132
Conversation
cc @djlikesdjs |
@navidcy can you please include the link to the docs preview page so that we see how it looks before approving? |
Could you build the docs locally? Comment out all examples here Lines 30 to 43 in 72a2399
except the tide one and run:
and then open, e.g., If you even change Lines 141 to 151 in 72a2399
to doctest = false , strict = false and checkdocs = :none , it'll speed things even more.
(It took ~8 mins on my laptop.) |
I know how to do it, but I didn't try. I thought the best practice was to set But fair enough. I'll build and view them locally for review. |
examples/barotropic_tide.jl
Outdated
hill(x) = h₀ * exp(-x^2 / 2width^2) | ||
bottom(x, y) = - H + hill(x) | ||
|
||
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom)) |
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.
This example is probably the perfect test for PartialCellBottom
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.
Yes, or even better for shaved cells!
I'd be very interested to see if these streaks of w-velocities in the initial condition diminish with shaved cells.
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.
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.
Why don't we use PartialCellBottom
here?
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.
When I tried to change
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
to
using Oceananigans.ImmersedBoundaries: PartialCellBottom
grid = ImmersedBoundaryGrid(underlying_grid, PartialCellBottom(bottom))
the simulation NaN
ed immediately. Even when I reduced the max_Δt
from 6 minutes down to 2 minutes..
We can. We can also mask the output after we load it, e.g, via using something like using Oceananigans.ImmersedBoundaries: mask_immersed_field!
function mask_and_get_interior(φ_t, n; value=NaN)
mask_immersed_field!(φ_t[n], value)
return interior(φ_t[n], :, 1, :)
end
u′ₙ = @lift mask_and_get_interior(u′_t, $n) which gives internal_tide.mp4But either of these solutions complicate the example a bit. Ideally, |
But I wouldn't mind adding the post-processing masking step with the hope we can eliminate it when masking is enabled for OutputWriters; see #3092 |
The movie looks great and I'm happy to see this example coming to life! One question I have is in the plot for stratification, at the very top. It looks like there is a black line and then there is a thin layer right below that. Anyone know what that is? |
I agree with @simone-silvestri that we want this feature, but that it should be done properly |
I think we should mask the solution during visualization in the script. This is actually what users will need to do currently to make decent visualizations, so it is good to illustrate how to do it --- even if, hopefully, we will eventually have a better solution. As for plotting a shape on top, I'm inclined to encourage visualization that directly represents the domain / data for the purposes of an Oceananigans example. I think its ok if people want to incorporate visualizations like that in their own work but we may not want to promote it as "the best" way to visualize complex domains in this example. |
I also agree. That’s how the example is now. If you approve then please approve? |
Masking is done: Oceananigans.jl/examples/internal_tide.jl Lines 213 to 217 in ca43714
|
@francispoulin nice catch! using Oceananigans
Nx, Nz = 25, 12
underlying_grid = RectilinearGrid(size = (Nx, Nz), x = (-1000e3, 1000e3), z = (-2000, 0),
halo = (4, 4), topology = (Periodic, Flat, Bounded))
h₀, width = 250, 20e3
bottom(x) = - H + h₀ * exp(-x^2 / 2width^2)
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = HydrostaticFreeSurfaceModel(; grid, buoyancy = BuoyancyTracer(), tracers = :b)
Nᵢ² = 1e-4
bᵢ(x, z) = Nᵢ² * z
set!(model, b=bᵢ)
b = model.tracers.b
N² = Field(∂z(b))
compute!(N²)
using GLMakie
xb, _, zb = nodes(b)
fig = Figure()
ax = Axis(fig[1, 1])
hm = heatmap!(ax, xb, zb, interior(b, :, 1, :))
Colorbar(fig[1, 2], hm)
save("b.png", current_figure()) xN², _, zN² = nodes(N²)
fig = Figure()
ax = Axis(fig[1, 1])
hm = heatmap!(ax, xN², zN², interior(N², :, 1, :), colorrange = (0, 2Nᵢ²))
Colorbar(fig[1, 2], hm)
save("N2.png", current_figure()) I don't know what would be the solution to this. I think it's physical in our domain to have no flux condition of |
Makes sense. In this case would it be better to specify a non zero flux on the buoyancy, the ambient stratification? It won't change the nice results you are getting over the topography but makes sense to me. No need to make this change. |
I think the proper way to do this is to use BackgroundField for the ambient stratification. I’ll give it a go |
Hm.... |
That's great. |
Right, we do not support that right now. I think there are some niche uses but, for the most part, when we go hydrostatic we are often doing large scale and a bit more complicated modeling with parameterizations, etc, where getting background fields right would be more of a pain than with NonhydrostaticModel (which still does not work properly with nonlinear subgrid-scale turbulence closures). I think its better to go more mainstream for this example. I think the use cases that this example serves as a starting point for are probably not going to use background fields, typically. |
True true. Any idea how to deal with the dz(b) that vanishes at the top? Or should we live with it? |
Because of the no-flux boundary condition? I think that's correct, ok to have. You could have a mixed layer at the top too! |
hm... I don't know about a mixed layer... let's just ignore the dz(b)=0 at the top&bottom or we can add a remark in the example about it? |
Yes definitely ignore. What we want to focus on is providing an example that is a useful starting point for other, new simulations. This is the key priority. This is why it makes sense to prioritize relatively simple examples (and also simple visualization) that can be quickly digested and are amenable to generalization. We want to deprioritize bespoke or niche cases that are a dead-end. For example, if we use |
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.
This is a great start and I think in the future we can think about adding bottom drag using ImmersedBoundaryCondition
This is a simple example with immersed boundary.
barotropic_tide.mp4