-
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
Avoid allocations due to usage of broadcasting operation #1656
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. |
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! Did you also have the bandwidth to investigate what's going on? We had some problems like this already earlier, e.g., #1626
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.
Agreed with Hendrik's suggestions.
Can you also make a note that the loops were introduced to avoid allocations due to broadcasting? If it's possible to link to the test failure that would also be good.
Hopefully we can eventually figure out what is causing this.
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1656 +/- ##
=======================================
Coverage 96.11% 96.11%
=======================================
Files 418 418
Lines 34238 34240 +2
=======================================
+ Hits 32907 32909 +2
Misses 1331 1331
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Not really - this is quite a mystery to me, as Trixi.jl/src/solvers/dgmulti/dg.jl Line 305 in adce486
is allocating while Trixi.jl/src/solvers/dgmulti/dg.jl Line 330 in adce486
is not... |
This caused CI tests to fail, see for instance https://github.com/trixi-framework/Trixi.jl/pull/1650/checks