Allow a-posteriori corrections for du#2953
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. |
|
I tested the resulting numbers of |
|
Problem: The amount of limiting depends on the time step. We don't have a fully semidiscrete scheme. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2953 +/- ##
==========================================
- Coverage 97.11% 92.12% -5.00%
==========================================
Files 622 622
Lines 48235 48249 +14
==========================================
- Hits 46843 44446 -2397
- Misses 1392 3803 +2411
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:
|
amrueda
left a comment
There was a problem hiding this comment.
Thanks, @bennibolm... And sorry for the delay!
This is a nice and elegant fix to the problem.
LGTM... But there are some tests failing.
| # Apply `perform_idp_correction!` to `du` instead of `u`: | ||
| # Pass `du` and `1.0` instead of `u` and `dt` | ||
| perform_idp_correction!(du, 1.0, mesh, equations, solver, cache) |
There was a problem hiding this comment.
| # Apply `perform_idp_correction!` to `du` instead of `u`: | |
| # Pass `du` and `1.0` instead of `u` and `dt` | |
| perform_idp_correction!(du, 1.0, mesh, equations, solver, cache) | |
| # Apply `perform_idp_correction!` to `du` instead of `u`: | |
| # Pass `du` and `1.0` instead of `u` and `dt`. This returns the correct | |
| # semi-discrete correction: du_i = du_i^(FV) + \sum_j f^(antidiffusive)_ij | |
| perform_idp_correction!(du, 1.0, mesh, equations, solver, cache) |
amrueda
left a comment
There was a problem hiding this comment.
On a second pass, I realized the following:
At each analysis callback, du is computed and subsequently "corrected" with the IDP routines using the bounds that were computed for the explicit Euler step of the previous RK stage. This is a bit weird because there is no guarantee that those are meaningful bounds for the solution at the time of the analysis callback. Also, there is no guarantee that the low-order method can fulfill those bounds. I'd suggest computing the bounds before du_correction_subcell_limiting!.
| return nothing | ||
| end | ||
|
|
||
| # `SubcellLimiterIDPCorrection` is the first entry in the tuple of stage callbacks. |
There was a problem hiding this comment.
Is this always the case? if not, we'll have to do something different here or force it to be the first.
| @notimeit timer() integrator.f(du_ode, u_ode, semi, t) | ||
| # Update `du` with corrections from `SubcellLimiterIDPCorrection` for a-posteriori subcell limiting | ||
| du_correction_subcell_limiting!(du_ode, u_ode, integrator, | ||
| integrator.p.solver.volume_integral) |
There was a problem hiding this comment.
I think that we have to re-compute the bounds before the call to du_correction_subcell_limiting!. The most consistent way to do this is, probably, to force the call of the analysis callback to be after the call to the time-step callback...
Oh yeah, you are right. In that case, I actually would prefer to close this PR and don't add additional computations. As we discussed back then, the IDP limiting is simply not a pure semi-discrete method and the entropy analysis doesn't make full sense. |
See #2948.
To fix this, I added a
SubcellLimiterIDPCorrectionfunction which correctsduaccordingly.Additionally, I added a check whether
SubcellLimiterIDPCorrectionis the first stage callback in the custom SSP time integration method. It doesn't make sense if it's not.