Covariant formulation of the shallow water equations #58
Conversation
…m/new_shallow_water
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
+ Coverage 89.45% 90.37% +0.92%
==========================================
Files 20 21 +1
Lines 1802 2099 +297
==========================================
+ Hits 1612 1897 +285
- Misses 190 202 +12 ☔ View full report in Codecov by Sentry. |
tristanmontoya
changed the title
There was a problem hiding this comment.
Many thanks, @tristanmontoya! great work!
I have not checked the math thoroughly, but I leave a couple of comments below.
BTW, thanks for improving the documentation of my elixirs as well!
| # Since the standard weak form for CovariantShallowWaterEquations2D has no nonconservative | ||
| # terms, but the equation type has have_nonconservative_terms = True(), we must define | ||
| # an appropriate kernel that just calls the standard weak form | ||
| @inline function Trixi.weak_form_kernel!(du, u, element, mesh::P4estMesh{2}, | ||
| nonconservative_terms::True, | ||
| equations::AbstractCovariantEquations{2}, | ||
| dg::DGSEM, cache, alpha = true) | ||
| Trixi.weak_form_kernel!(du, u, element, mesh, False(), equations, dg, cache, alpha) | ||
| end |
There was a problem hiding this comment.
I understand why you need this function (I am having the same problem in PR #53), but I think that we should include a computation of the non-conservative terms here. Otherwise, someone might try to use a non-conservative equation with the weak form kernel in the future and get a wrong result, but no dispatch error..
There was a problem hiding this comment.
Alternatively, you could use the flux_differencing_kernel! with a central flux. I think this won't be equivalent to the weak-form kernel, but maybe similar(?)
There was a problem hiding this comment.
Good point. I want to have a formulation that is exactly equivalent to the weak form as a baseline. This can be done using an equivalent flux-differencing form, but the nonconservative flux will return zero unless there is bottom topography. That's probably a better solution (and it won't return zero once we add the bottom topography).
| # Our implementation of flux-differencing formulation uses nonconservative terms, but the | ||
| # standard weak form does not. To handle both options, we have defined a dummy kernel for | ||
| # the nonconservative terms that does nothing when VolumeIntegralWeakForm is used with a | ||
| # nonconservative system. |
There was a problem hiding this comment.
TODO: Modify this comment if the weak-form kernel changes.
src/solvers/dgsem_p4est/containers_2d_manifold_in_3d_covariant.jl
Outdated
Show resolved
Hide resolved
| # Entropy-conservative flux with local Lax-Friedrichs dissipation | ||
| const flux_ec_llf = FluxPlusDissipation(flux_ec, | ||
| DissipationLocalLaxFriedrichs(max_abs_speed_naive)) |
There was a problem hiding this comment.
Is this provably ES or has any special property? For the Cartesian SW or Euler equations, we typically use the "standard" LLF with flux_central and DissipationLocalLaxFriedrichs, or the EC-LLF with flux_ec and a dissipation operator in terms of the Jump in entropy variables (only recently implemented in Trixi: trixi-framework/Trixi.jl#2204). Would the latter approach be better here?
There was a problem hiding this comment.
This makes me realize that the "standard" (flux_central) approach does not work here as flux_ec is not consistent with flux... Can that be a problem?
There was a problem hiding this comment.
I am not 100% sure if it's provably entropy stable with this dissipation term, but I am also not sure about the standard LLF for this problem (we would have to do the theoretical analysis). The entropy variable jump approach would be my preference, but haven't looked into how straightforward the implementation would be. I will also look into what needs to be done to use the standard LLF here.
New version of #37 with entropy-stable split forms and auxiliary variables. Requires Trixi.jl v0.9.9 (see #59). The following new features are implemented:
CovariantShallowWaterEquations2Dwhich implements the split formulation of the covariant derivative using non-conservative terms, but does not use non-conservative terms for the standard formulation. The split formulation conserves total energy when no interface dissipation is added.func(u, aux_vars, equations)rather than justfunc(u, equations), which is needed sinceentropyis a function of auxiliary variables in this case.Jrather thansqrtGfor the area element.