-
Notifications
You must be signed in to change notification settings - Fork 134
First PR for 3D subcell limiting #2582
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
First PR for 3D subcell limiting #2582
Conversation
Review checklistThis checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging. Purpose and scope
Code quality
Documentation
Testing
Performance
Verification
Created with ❤️ by the Trixi.jl community. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2582 +/- ##
==========================================
+ Coverage 96.83% 96.84% +0.01%
==========================================
Files 540 544 +4
Lines 42885 43145 +260
==========================================
+ Hits 41527 41782 +255
- Misses 1358 1363 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I will take a closer look at the code this afternoon. Could you maybe run one more refinement step of the convergence test (If it takes not too much time)? |
Co-authored-by: Daniel Doehring <[email protected]>
|
We should keep an eye on #2632 Maybe it makes sense to define abstract struct AbstractContainerAntidiffusiveFlux <: AbstractContainer end
nvariables(fluxes::AbstractContainerAntidiffusiveFlux) = size(fluxes.antidiffusive_flux1_L, 1)
nnodes(fluxes::AbstractContainerAntidiffusiveFlux) = size(fluxes.antidiffusive_flux1_L, 3)which is the same as Trixi.jl/src/solvers/dgsem_tree/containers_2d.jl Lines 1290 to 1291 in b5ecc2a
Then mutable struct ContainerAntidiffusiveFlux2D{uEltype <: Real} <: AbstractContainerAntidiffusiveFlux
endto make the code slightly more self-documenting? |
amrueda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together, @bennibolm!
A couple of minor comments:
amrueda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
There are some CI failures that do not seem to be related...
|
Because of the unrelated CI failure, we'd need a force push. Can you do it @amrueda |
This PR contains 3D subcell limiting for
P4estMesh:Convergence test with polynomial degree of 4:
used the following mesh

Example of the sedov blast (adapted from the 3d blast in the new elixir to mimic a 2d blast) : I used a easier setup with pressure 0.1 since only positivity limiting of density can be used.
On the left: Simulation in 3d. I adapted the initial condition such that nothing happens in the third direction.
On the right: Simulation in 2d
video.mp4
There are very small differences between the simulations, but I would say that this is expected.