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.

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

Generalize ErtelPotentialVorticity and RichardsonNumber to work when model.buoyancy is not BuoyancyTracer #190

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

tomchor
Copy link
Owner

@tomchor tomchor commented Dec 18, 2024

Easy fix. I wasn't aware that the function Oceananigans.BuoyancyFormulations.buoyancy existed when I first coded EPV.

src/FlowDiagnostics.jl Outdated Show resolved Hide resolved
@tomchor tomchor requested a review from glwagner December 19, 2024 02:40
@tomchor tomchor changed the title Generalize ErtelPotentialVorticity to work when model.buoyancy is not BuoyancyTracer Generalize ErtelPotentialVorticity and RichardsonNumber to work when model.buoyancy is not BuoyancyTracer Dec 19, 2024
@tomchor
Copy link
Owner Author

tomchor commented Dec 19, 2024

@glwagner I'd like your opinion on something. In a few of the CI runs, I get errors like these with the LagrangianAveragedDynamicSmag:

image

Where the two numbers are small, but differently small. Generally one of them is order 1e-16 and other is 1e-30. It doesn't happen every time, but often enough that I've noticed. That's what made the last CI run fail btw. Specifically this is the test that failed this previous time:

if model isa NonhydrostaticModel
χ = TracerVarianceTendency(model, :b)
χ_field = compute!(Field(χ))
@test χ isa AbstractOperation
@test χ_field isa Field
@compute ∂ₜc² = Field(Average(TracerVarianceTendency(model, :b)))
@test (Array(interior(ε̄ₚ, 1, 1, 1)), -Array(interior(∂ₜc², 1, 1, 1)), rtol=1e-10, atol=2*eps())
end

Which compares the KE dissipation from the closure model with the total tendency. Since all that's there is the closure model and no other tendencies, they should be the same. I know that eps(Float64) ~ 1e-16 so I'm wondering if this is just natural uncertainty due to limited machine precision, of if there's something else at play here (since the tolerance is already 2*eps() for that test).

Do you have thoughts?

EDIT: FYI I just restarted that run. So if things are passing now, it's because of the normal variability in these tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants