Avoid allocations in DGMulti parabolic calc_boundary_flux!#2947
Conversation
Replace recursive first/Base.tail iteration over boundary NamedTuples with the same zip(keys, values) loop used by hyperbolic DGMulti (dg.jl). This removes heap allocations on rhs_parabolic! for dgmulti advection-diffusion setups. Fixes trixi-framework#2729 Made-with: Cursor
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2947 +/- ##
==========================================
- Coverage 97.13% 97.13% -0.00%
==========================================
Files 623 623
Lines 48385 48384 -1
==========================================
- Hits 46996 46995 -1
Misses 1389 1389
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:
|
DanielDoehring
left a comment
There was a problem hiding this comment.
Given that this is the same implementation as for the hyperbolic case I am fine with this change.
|
Thanks for the review! |
|
@ranocha OK if I merge assuming tests pass? |
Human summary
Some manual testing showed that removing the recursive call to
calc_boundary_flux!Trixi.jl/src/solvers/dgmulti/dg_parabolic.jl
Lines 249 to 250 in ea79220
appeared to fix the excess allocations observed in #2729. This PR changes the implementation of
calc_boundary_flux!to look more like the hyperbolic case. I'm not sure why I didn't consider this option earlier 🤷This produces qualitatively correct solution plots for
examples/dgmulti_2d/elixir_advection_diffusion.jlexamples/dgmulti_2d/elixir_navierstokes_lid_driven_cavity.jl.AI summary
Parabolic
calc_boundary_flux!forDGMultipeeledNamedTupleboundary conditions withfirst/Base.tailrecursion, which caused heap allocations inrhs_parabolic!(for exampleexamples/dgmulti_2d/elixir_advection_diffusion.jl).This change matches the hyperbolic DGMulti implementation in
dg.jl: iterate withzip(keys(boundary_conditions), boundary_conditions)and callcalc_single_boundary_flux!per segment. The empty-NamedTupleterminator is removed.Verification
After
trixi_include("examples/dgmulti_2d/elixir_advection_diffusion.jl"),@allocated Trixi.rhs_parabolic!(du, ode.u0, semi, 0.0)reports zero bytes (post warm-up).Fixes #2729
Made with Cursor