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.

Sign up for GitHub

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

Jump to bottom

Bug in ShallowWaterModel treatment of bathymetric contribution to pressure #3051

Open
glwagner opened this issue Apr 5, 2023 · 3 comments · May be fixed by #3502
Open

Bug in ShallowWaterModel treatment of bathymetric contribution to pressure #3051

glwagner opened this issue Apr 5, 2023 · 3 comments · May be fixed by #3502
Labels
bug 🐞 Even a perfect program still has bugs numerics 🧮 So things don't blow up and boil the lobsters alive

Comments

@glwagner
Copy link
Member

glwagner commented Apr 5, 2023 •
edited

These lines multiply h (a field at ccc) with objects at fcc and cfc respectively

Oceananigans.jl/src/Models/ShallowWaterModels/solution_and_tracer_tendencies.jl

Lines 16 to 17 in bcc34f0

@inline bathymetry_contribution_x(i, j, k, grid, g, h, hB, formulation) = g * h[i, j, k] * ∂xᶠᶜᶜ(i, j, k, grid, hB)
@inline bathymetry_contribution_y(i, j, k, grid, g, h, hB, formulation) = g * h[i, j, k] * ∂yᶜᶠᶜ(i, j, k, grid, hB)

This doesn't look correct, but if there is some logic that makes it correct, it should be documented with a comment at least. I think if the bathymetric height is defined at cell centers, then the bathymetric height at a cell interface might need to be defined as the maximum of the height of the adjacent cells.
@glwagner glwagner added bug 🐞 Even a perfect program still has bugs numerics 🧮 So things don't blow up and boil the lobsters alive labels Apr 5, 2023
@navidcy
Copy link
Collaborator

navidcy commented Apr 5, 2023 •
edited

Agree!

[Btw, these lines could use an @inbounds :)]

@navidcy navidcy changed the title Bug in ShallowWaterModel treatment of bathymetric contribution to pressure Bug in ShallowWaterModel treatment of bathymetric contribution to pressure Apr 5, 2023
@glwagner
Copy link
Member Author

glwagner commented Apr 5, 2023

Another issue.

@simone-silvestri
Copy link
Collaborator

This definitely seems wrong to me, the bathymetric height is defined at cells centers, so we should interpolate h to velocity points

@navidcy navidcy mentioned this issue Mar 3, 2024
Closed
@francispoulin francispoulin linked a pull request Mar 7, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐞 Even a perfect program still has bugs numerics 🧮 So things don't blow up and boil the lobsters alive
Projects
None yet
Milestone
No milestone
3 participants
@navidcy @glwagner @simone-silvestri