-
Couldn't load subscription status.
- Fork 131
Adaptive Volume Integral #2601
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
base: main
Are you sure you want to change the base?
Adaptive Volume Integral #2601
Conversation
…ixi.jl into VolumeIntegralAdaptive
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 #2601 +/- ##
==========================================
- Coverage 96.80% 96.80% -0.00%
==========================================
Files 530 531 +1
Lines 42719 42817 +98
==========================================
+ Hits 41354 41448 +94
- Misses 1365 1369 +4
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:
|
| Possible combination: [`VolumeIntegralWeakForm`](@ref) and [`VolumeIntegralFluxDifferencing`](@ref). | ||
| TODO: Extend to [`VolumeIntegralWeakForm`](@ref) and [`VolumeIntegralShockCapturingHG`](@ref). |
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.
There are some TODO notes left. How do you want to handle them?
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.
This is probably something for the Trixi meeting. The main question is if VolumeIntegralShockCapturingHG should be (internally) re-implemented as a VolumeIntegralAdaptive.
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.
👍
| """ | ||
| VolumeIntegralAdaptive(indicator; | ||
| volume_integral_default = VolumeIntegralWeakForm(), | ||
| volume_integral_stabilized = VolumeIntegralFluxDifferencing(flux_central)) |
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.
Shall we really prove this as default value? Or should we force people to make a reasonable choice?
| decision = @trixi_timeit timer() "integral selector" indicator(u, mesh, equations, | ||
| dg, cache) |
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.
| decision = @trixi_timeit timer() "integral selector" indicator(u, mesh, equations, | |
| dg, cache) | |
| decision = @trixi_timeit timer() "blending factors" indicator(u, mesh, equations, | |
| dg, cache) |
For consistency with the Hennemann-Gassner shock capturing?
| @threaded for element in eachelement(dg, cache) | ||
| stabilized_version = decision[element] | ||
|
|
||
| # TODO: Generalize/Dispatch or introduce yet sub-functions of the volume integrals |
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.
Good question
| @doc raw""" | ||
| IndicatorEntropyViolation(basis; | ||
| entropy_function=entropy, threshold=1e-9) |
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.
Do you have a reference for this idea?
| @unpack entropy_function, threshold = indicator_entropy_violation | ||
|
|
||
| # Beginning of simulation or after AMR: Need to compute `entropy_old` for every element | ||
| if length(entropy_old) != nelements(dg, cache) |
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.
Technically, AMR could be called, but the number of elements does not change, e.g., if one element is refined but another set of children is coarsened. In this case, the old entropy may be entirely off.
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.
True, that is a problem.
Co-authored-by: Hendrik Ranocha <[email protected]>
Having merged #2488 one could realize #2472 and (subsequently) #2149 via this. In fact,
VolumeIntegralShockCapturingHGcould also be brought into this framework.The benefit is greatly reduced computation time of the volume integral, more profound for higher polynomial degrees. For the examples employed in this (draft) PR, the volume integral computation can be roughly reduced by a factor of two.