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

weak_form_kernel! implemented only for nonconservative_terms::False #1671

Closed
DanielDoehring opened this issue Oct 16, 2023 · 6 comments · Fixed by #1676
Closed

weak_form_kernel! implemented only for nonconservative_terms::False #1671

DanielDoehring opened this issue Oct 16, 2023 · 6 comments · Fixed by #1676

Comments

@DanielDoehring
Copy link
Contributor

Across solvers, weak_form_kernel! is only implemented for nonconservative_terms::False, i.e.,

@inline function weak_form_kernel!(du, u,
element, mesh::Union{TreeMesh{1}, StructuredMesh{1}},
nonconservative_terms::False, equations,
dg::DGSEM, cache, alpha = true)

while the calling function

function calc_volume_integral!(du, u,
mesh::Union{TreeMesh{1}, StructuredMesh{1}},
nonconservative_terms, equations,
volume_integral::VolumeIntegralWeakForm,
dg::DGSEM, cache)

suggests that there is an implementation for nonconservative_terms::True at least possible.
Is this true, or can this particular volume integral not be computed in a meaningful way for non-conservative terms?

@ranocha
Copy link
Member

ranocha commented Oct 16, 2023

It could in principle but there is probably not much to be gained since we would like to use some split form anyway - but you may better ask our nonconservative wizard @andrewwinters5000

@andrewwinters5000
Copy link
Member

andrewwinters5000 commented Oct 17, 2023

In principle one can probably derive and implement the non-conservative terms in a weak form style. But, as @ranocha pointed out, we do not really have a use case for this, as we typically need a split form anyway for entropy stability or for well-balancedness in the case of the shallow water equations. There are also some subtleties that arise if you want to do the non-conservative term of the shallow water equations weakly. What I mean is given a test function $\phi$ then the inner product form after one application of integration-by-parts would be $$(ghb_x,\phi) = ghb\phi|_{\partial E} - (gb, (h\phi)_x)$$
because the weak derivative now falls on the product $h\phi$ instead of $b$, deriving a well-balanced approximation becomes tricky. Not to say it is impossible, I just have never investigated it further.

@DanielDoehring
Copy link
Contributor Author

Okay, interesting! Maybe it makes sense to add a comment explaining why there is only one implementation available.

@ranocha
Copy link
Member

ranocha commented Oct 17, 2023

Please feel free to make a PR with such a comment, linking to this discussion

@DanielDoehring
Copy link
Contributor Author

Opinions on this? @ranocha @andrewwinters5000

"""
`weak_form_kernel!` is only implemented for conserved terms as 
non-conserved terms are supposed to be always applied in conjunction with a flux-splitting scheme, 
see `flux_differencing_kernel!`.
This treatment is required to achieve e.g. entropy-stability and well-balancedness.
"""

@ranocha
Copy link
Member

ranocha commented Oct 18, 2023

Something like that could work as a comment for the weak form kernel itself - wuth a link to this issue. You can also mention something similar in the docstring of the volume integral type - we should have a docstring for it, don't we?

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 a pull request may close this issue.

3 participants