Add auxiliary variables, part 1#3008
Conversation
- add container for node and surface auxiliary variables - add initializer to SemidiscretizationHyperbolic - dispatch on have_auxiliary_node_vars for 2D TreeMesh rhs!
to demonstrate how auxiliary variables can be used instead of augmenting the solution state vector
ŕesults have to be identical to the corresponing AcousticPerturbationEquations2D results
|
All conflicts resolved. This is now ready for review. Downstream tests are expected to fail because TrixiShallowWater.jl and TrixiAtmo.jl overwrite internal Trixi.jl functions, e.g. https://github.com/trixi-framework/TrixiShallowWater.jl/blob/14552f2fdd0df83dbc094d2700c9e37dfaeab037/src/solvers/dgsem_p4est/dg_2d.jl#L314 would now need an additional |
There was a problem hiding this comment.
Thanks Benedict! That looks so interesting and great to have in Trixi. I have not made a review, but just left a few comments to understand better the infrastructure. I'm not actually sure if there are some problems or issues that could increase the complexity of the implementation.
| ############################################################################### | ||
| # semidiscretization of the acoustic perturbation equations | ||
|
|
||
| function auxiliary_variables_mean_values(x, equations) |
There was a problem hiding this comment.
Would it make sense (or be interesting) to compute first the initial condition and then pass also the initial condition u0 as a parameter to the auxiliary field function?
There was a problem hiding this comment.
I see this would be convenient when e.g. setting up perturbations of a steady background state.
However, u0 is computed using compute_coefficients(t, semi), which takes a Semidiscretization object, right?
|
I suggest this splitting into multiple PRs, starting from 1D |
ranocha
left a comment
There was a problem hiding this comment.
Thanks for your efforts. How do you see the chances to allow discontinuities at interfaces?
| # constant auxiliary variables (mean state) | ||
| return global_mean_vars(equations) | ||
| end |
There was a problem hiding this comment.
Does it really make sense to introduce this set of equations if the auxiliary variables are only given by some constants that are also stored in the equations? Could it be better to use something that appears to be less artificial, e.g., a 2D linear advection equation with variable velocity field? If it is required to be divergence free, a reasonable discretization can be a split form (but we can test the weak form as well, of course).
There was a problem hiding this comment.
This is indeed not really useful; merely a proof of concept that can be compared to something in main as a sanity check.
Non trivial velocity fields would be more exciting. I like the swirling flow by LeVeque:

Although in this form it requires time dependent aux vars, which I added quick and dirty here
Trixi.jl/src/solvers/dgsem_tree/dg_2d.jl
Lines 125 to 128 in 9eea80c
One could of course start with a time constant variant of it.
There was a problem hiding this comment.
Nice example! Something like this with constant-in-time (but variable-in-space) velocity would already be nice to ensure that the non-constant case works as expected.
There was a problem hiding this comment.
Done in examples/tree_2d_dgsem/elixir_advection_variable_swirling_flow.jl
| function analyze(quantity, du, u, t, semi::AbstractSemidiscretization) | ||
| mesh, equations, solver, cache = mesh_equations_solver_cache(semi) | ||
| return analyze(quantity, du, u, t, mesh, equations, solver, cache) | ||
| return analyze(quantity, du, u, t, mesh, have_aux_node_vars(equations), equations, | ||
| solver, cache) | ||
| end | ||
| function analyze(quantity, du, u, t, mesh, equations, solver, cache) | ||
| function analyze(quantity, du, u, t, mesh, have_aux_node_vars, equations, solver, cache) | ||
| return integrate(quantity, u, mesh, equations, solver, cache, normalize = true) | ||
| end |
There was a problem hiding this comment.
Do we acutally need the dispatch at this high level? It introduces some complexity here where I do not see the benefits of it. Would it be sufficient to extract the trait when it is required, e.g., for analyze(::typeof(entropy_timederivative), ... later?
There was a problem hiding this comment.
I'm not sure.
I need the aux vars here:
Trixi.jl/src/callbacks_step/analysis_dg2d.jl
Lines 472 to 485 in 8909f98
Would I need to specialize integrate_via_indices and the anonymous function (defined by the do syntax). So far, I was not successful.
There was a problem hiding this comment.
Can't you introduce the additional dispatch only for this method of analyze, e.g., something along the lines of
function analyze(::typeof(entropy_timederivative), du, u, t,
mesh::Union{TreeMesh{2}, StructuredMesh{2}, StructuredMeshView{2},
UnstructuredMesh2D, P4estMesh{2}, T8codeMesh{2}},
equations, dg::DG, cache)
# The entropy_timederivative may depend on auxiliary variables.
return analyze(entropy_timederivative, du, u, t,
mesh,
have_aux_node_vars(equations), equations,
dg, cache)
endand then the specialization you wrote above?
There was a problem hiding this comment.
Yes, obviously... Thanks for the hint!
Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
…k/Trixi.jl into bg/feature-aux-vars-merge
benegee
left a comment
There was a problem hiding this comment.
Thanks a lot for your feedback! I could easily address most of the comments.
How do you see the chances to allow discontinuities at interfaces?
This is something to discuss!
I think this would be possible as well.
- We can still define the interface as we see fit. This is the current call:
Trixi.jl/src/solvers/dgsem_tree/containers.jl
Line 199 in 9eea80c
We could additionally pass the element. Or how would we usually "detect" an interface. How do we do it with initial conditions? - So far I used the assumption the aux vars be continuous to spare some MPI communication at the interfaces, and to use non-interpolated data at mortar interfaces (comment in containers_2d), but this could be adapted appropriately.
| # constant auxiliary variables (mean state) | ||
| return global_mean_vars(equations) | ||
| end |
There was a problem hiding this comment.
This is indeed not really useful; merely a proof of concept that can be compared to something in main as a sanity check.
Non trivial velocity fields would be more exciting. I like the swirling flow by LeVeque:

Although in this form it requires time dependent aux vars, which I added quick and dirty here
Trixi.jl/src/solvers/dgsem_tree/dg_2d.jl
Lines 125 to 128 in 9eea80c
One could of course start with a time constant variant of it.
| function analyze(quantity, du, u, t, semi::AbstractSemidiscretization) | ||
| mesh, equations, solver, cache = mesh_equations_solver_cache(semi) | ||
| return analyze(quantity, du, u, t, mesh, equations, solver, cache) | ||
| return analyze(quantity, du, u, t, mesh, have_aux_node_vars(equations), equations, | ||
| solver, cache) | ||
| end | ||
| function analyze(quantity, du, u, t, mesh, equations, solver, cache) | ||
| function analyze(quantity, du, u, t, mesh, have_aux_node_vars, equations, solver, cache) | ||
| return integrate(quantity, u, mesh, equations, solver, cache, normalize = true) | ||
| end |
There was a problem hiding this comment.
I'm not sure.
I need the aux vars here:
Trixi.jl/src/callbacks_step/analysis_dg2d.jl
Lines 472 to 485 in 8909f98
Would I need to specialize integrate_via_indices and the anonymous function (defined by the do syntax). So far, I was not successful.
…k/Trixi.jl into bg/feature-aux-vars-merge
There was a problem hiding this comment.
The core implementation looks good, although if we can remove the trait and just use zero-length vectors without overhead like you mentioned here, that would probably be cleaner, assuming there's negligible overhead. My suggestions are basically just to add clearer documentation in places (see comments).
| function analyze(::typeof(entropy_timederivative), du, u, t, | ||
| mesh::Union{TreeMesh{1}, StructuredMesh{1}}, equations, | ||
| dg::Union{DGSEM, FDSBP}, cache) | ||
| mesh::Union{TreeMesh{1}, StructuredMesh{1}}, | ||
| equations, dg::Union{DGSEM, FDSBP}, cache) |
There was a problem hiding this comment.
There are some lines in this PR that just seem to be formatting changes. Not a big deal but it makes the diff look a bit bigger than it actually is.
| Trait function determining whether `equations` need to access additional auxiliary | ||
| variables. |
There was a problem hiding this comment.
I would give a more complete description of auxiliary variables here, or if you define it elsewhere, link it here. For example:
| Trait function determining whether `equations` need to access additional auxiliary | |
| variables. | |
| Trait function determining whether auxiliary variables should be stored at each node | |
| alongside the solution variables and used to compute the flux for `equations`. When | |
| `True()`, the signature of [`flux`](@ref) becomes the following: | |
| ``` | |
| flux(u, aux_vars, orientation_or_normal_direction, equations) | |
| ``` | |
| The auxiliary variables are computed by passing an additional keyword argument | |
| `aux_field` into [`SemidiscretizationHyperbolic`](@ref), where | |
| `aux_field(x, equations)` returns an `SVector` of size `n_aux_node_vars(equations)` | |
| at every point `x` in the spatial domain. | |
| !!! warning "Experimental implementation" | |
| The use of auxiliary variables is an experimental feature and may change in future | |
| releases. Currently, only 2D `TreeMesh` is supported. |
| which will be available, e.g., in flux computations. The current `equations` need to set | ||
| `have_aux_node_vars to `True()` and `n_aux_node_vars` to the number of auxiliary variables | ||
| per node. Upon refinement, `aux_field` will be called again to recompute the auxiliary variables. | ||
| NOTE: This is experimental! |
There was a problem hiding this comment.
| NOTE: This is experimental! | |
| !!! warning "Experimental implementation" | |
| The use of auxiliary variables is an experimental feature and may change in future | |
| releases. Currently, only 2D `TreeMesh` is supported. |
Continuing #2348
As a first step, auxiliary variables are implemented for TreeMesh 2D.
AcousticPerturbationEquations2DAuxVarsdemonstrates how auxiliary variables can be used instead of augmenting the solution state vector, as previously done, e.g., in AcousticPerturbationEquations2D.Some comments:
have_aux_node_vars(equations)and using the result for multiple dispatch.have_aux_node_vars::Trueare added only for the 2D TreeMesh implementation in this PR. The code changes are trivial, usually just addingget_aux_node_varsorget_aux_surface_node_varswhereget_node_varsorget_surface_node_varswere already called, and passing the resulting aux vars down, e.g. to flux computations.solvers/dgsem_tree/containers_2d.jl, although it mostly follows correspondingprolong2*functions.Supersedes #2907