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

Inconsistency in boundary conditions for nonconservative terms #2175

Open
andrewwinters5000 opened this issue Nov 22, 2024 · 8 comments
Open
Labels
bug Something isn't working enhancement New feature or request

Comments

@andrewwinters5000
Copy link
Member

When computing the boundary conditions for systems with nonconservative terms there is a potential to accidentally add the boundary contribution incorrectly on P4estMesh (https://github.com/trixi-framework/Trixi.jl/blob/main/src/solvers/dgsem_p4est/dg_2d.jl#L365-L371 ) or UnstructuredMesh2D (https://github.com/trixi-framework/Trixi.jl/blob/main/src/solvers/dgsem_unstructured/dg_2d.jl#L443-L449).

If one does not use the surface_flux_function or nonconservative_flux to compute the boundary condition, e.g. as we do for something like the Euler slip walls (https://github.com/trixi-framework/Trixi.jl/blob/main/src/equations/compressible_euler_2d.jl#L299-L337), then the boundary condition will incorrectly be added as something like

surface_flux_values = flux + 0.5 *  noncons

because flux and noncons will be identical and we actually get the boundary flux one and a half times.

It would be better from my perspective if we did not assume / require that the boundary condition is set via a numerical flux for the sake of flexibility.

@andrewwinters5000 andrewwinters5000 added bug Something isn't working enhancement New feature or request labels Nov 22, 2024
@andrewwinters5000
Copy link
Member Author

@ranocha I think this is related to issues that Marco discussed at the last Trixi meeting.

@andrewwinters5000
Copy link
Member Author

As a starting idea, it would be handy if the boundary condition routine(s) could use a different surface_flux than that used for the interior interfaces taken from surface_integral. This would already grant extra flexibility.

@ranocha
Copy link
Member

ranocha commented Nov 27, 2024

CC @MarcoArtiano

@ranocha
Copy link
Member

ranocha commented Nov 27, 2024

As discussed in the last meeting, we should consider passing (flux_conservative, flux_nonconservative) as flux and call the BC only once. This would need to be tested in practice and documented appropriately.

@MarcoArtiano
Copy link

MarcoArtiano commented Nov 27, 2024

Unfortunately I couldn't join the Trixi Meeting yesterday. My suggestion and my understanding would have been that I could theoretically use an anti-symmetric volume flux for the non-conservative part, that is consistent with 0, so that no boundary conditions needs to be defined for the non-conservative flux. As far as I know, every non-conservative formulation in Flux Differencing form can be written as a product of a symmetric flux times a jump (i.e. anti-symmetric flux) (at the moment I don't know of any non-conservative flux that doesn't boil down to this form. This is basically, an equivalent form to the one presented here, https://arxiv.org/pdf/2211.14009, where a non-conservative flux can be written as a product of a symmetric times a local term). The anti-symmetric "formulation" would be also consistent with the point-wise source term, where no boundary conditions are imposed. However, that would require to modify all the existing non-conservative fluxes and might not be the best idea at the moment.

Anyway, I can definitely work on the suggestion presented and pass the tuple directly to the boundary condition call and return the correct value flux + 0.5*cons.

@andrewwinters5000
Copy link
Member Author

Anyway, I can definitely work on the suggestion presented and pass the tuple directly to the boundary condition call and return the correct value flux + 0.5*cons.

That would be great! Feel free to ping me for a review whenever you have a PR ready.

@patrickersing
Copy link
Contributor

My suggestion and my understanding would have been that I could theoretically use an anti-symmetric volume flux for the non-conservative part, that is consistent with 0, so that no boundary conditions needs to be defined for the non-conservative flux.

Even with anti-symmetric non-conservative terms, these don't need to be consistent with zero at the boundary, since u_ll and u_rr may differ and the jump term is non-zero. So there are actually some cases where boundary conditions need to be imposed for the nonconservative terms.

@MarcoArtiano
Copy link

My suggestion and my understanding would have been that I could theoretically use an anti-symmetric volume flux for the non-conservative part, that is consistent with 0, so that no boundary conditions needs to be defined for the non-conservative flux.

Even with anti-symmetric non-conservative terms, these don't need to be consistent with zero at the boundary, since u_ll and u_rr may differ and the jump term is non-zero. So there are actually some cases where boundary conditions need to be imposed for the nonconservative terms.

You are definitely right! I didn't consider that option, it would not allow for such flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants