-
Notifications
You must be signed in to change notification settings - Fork 112
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
Increase type stab, avoid allocs #1642
Increase type stab, avoid allocs #1642
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. |
Furthermore, it looks like Trixi.jl/test/test_threaded.jl Line 318 in 73384ac
@test (@allocated Trixi.rhs!(du_ode, u_ode, semi, t)) < 5000 thus rendering the Trixi.jl/test/test_threaded.jl Lines 315 to 319 in 73384ac
obsolete. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1642 +/- ##
=======================================
Coverage 96.11% 96.11%
=======================================
Files 418 418
Lines 34247 34247
=======================================
Hits 32915 32915
Misses 1332 1332
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Could you please verify this locally (on Julia v1.8 and v1.9, serial and with multiple threads) and update the tests accordingly? |
…ing/Trixi.jl into TypeStabilityBCsUnstructured
On Ubuntu 23.04 this seems to work indeed for the four mentioned setups. |
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!
Similar to #1635 and the corresponding fix #1636 I noticed in recent test runs (e.g. https://github.com/trixi-framework/Trixi.jl/actions/runs/6219235479/job/16876951926?pr=1629#step:7:4212) allocations in
calc_boundary_flux!
.For https://github.com/trixi-framework/Trixi.jl/blob/main/examples/unstructured_2d_dgsem/elixir_acoustics_gauss_wall.jl I get for the current version on main
With the proposed changes