-
Notifications
You must be signed in to change notification settings - Fork 15
Clean up timer outputs #819
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?
Conversation
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.
Pull Request Overview
This PR aims to clean up timer outputs by grouping update calls into block scopes and using the @notimeit macro to prevent creating extraneous sub-timers. Key changes include:
- Wrapping update_systems_and_nhs calls in a begin…end block with @notimeit in I/O, post-process, and particle shifting callbacks.
- Updating timer label names to reflect the encompassed operations.
- Adjusting the TimerOutputs import to include the new @notimeit macro.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/io/write_vtk.jl | Wrapped update_systems_and_nhs call in a block to suppress sub-timer output. |
src/callbacks/post_process.jl | Similar block wrapping applied with an updated timer label for clarity. |
src/callbacks/particle_shifting.jl | Nested timer blocks introduced to group update operations and prevent clutter. |
src/TrixiParticles.jl | Updated TimerOutputs import to include the new @notimeit macro. |
Comments suppressed due to low confidence (2)
src/callbacks/post_process.jl:241
- [nitpick] Consider standardizing the timer label names across modules. In src/io/write_vtk.jl the timer label remains 'update systems', whereas here it is 'update systems and nhs'. Choose one consistent label to avoid confusion.
@trixi_timeit timer() "update systems and nhs" begin
src/callbacks/particle_shifting.jl:40
- [nitpick] Ensure the timer label 'update systems and nhs' is used consistently with similar timer blocks elsewhere, or adjust it if a different naming convention is intended.
@trixi_timeit timer() "update systems and nhs" begin
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
==========================================
- Coverage 70.74% 70.70% -0.05%
==========================================
Files 106 106
Lines 6710 6714 +4
==========================================
Hits 4747 4747
- Misses 1963 1967 +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:
|
update_systems_and_nhs!
outside an@timeit
, which caused orphaned update timers likeupdate density diffusion
on the top level.kick!
), so there is no point in having all the sub-timers. Especially with Split fluid and TLSPH time integration to allow for TLSPH substeps #794, the timer output otherwise becomes very cluttered when multiple callbacks are used.Before:
This PR: