-
Notifications
You must be signed in to change notification settings - Fork 113
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
use finalize to print summary callback at end of simulation #2275
base: main
Are you sure you want to change the base?
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2275 +/- ##
==========================================
- Coverage 96.88% 96.88% -0.00%
==========================================
Files 490 490
Lines 39491 39492 +1
==========================================
- Hits 38260 38259 -1
- Misses 1231 1233 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Nice, thanks! I turned this into a draft PR so that nobody accidentally merges it. I needs a final review before being merged, of course.
The downgrade failure is not really related to this PR, but due to a new version of OrdinaryDiffEqSimplecticRK.jl being released, which leads to a failing downgrade job. If I understand correctly, a way to fix this, is bumping the compat of OrdinaryDiffEq.jl in tests/Project.toml to v6.91.0 (we need this commit, which first appeared in this version). However, fixing this here probably doesn't make so much sense as this PR will probably not be merged soon. I propose to fix this in another PR. |
Co-authored-by: Hendrik Ranocha <ranocha@users.noreply.github.com>
Xref SciML/OrdinaryDiffEq.jl#2600 for failing downgrade CI |
This uses the
finalize
keyword of theDiscreteCallback
to automatically print the output of theSummaryCallback
at the end of each simulation. This makes it unnecessary to manually callsummary_callback
at the end. Since seeing the output twice would be really annoying, this is considered breaking.Closes #1783.
For potential future reference, I used the following code to automatically remove the `summary_callback` calls in all the elixirs