-
Notifications
You must be signed in to change notification settings - Fork 115
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
Taal: AMR allocations #228
Taal: AMR allocations #228
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #228 +/- ##
==========================================
+ Coverage 89.59% 89.66% +0.06%
==========================================
Files 60 60
Lines 10485 10555 +70
==========================================
+ Hits 9394 9464 +70
Misses 1091 1091
Continue to review full report at Codecov.
|
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.
I think this generally looks good to me, and great work on reducing allocations 💪 I think it would be good to add more commentary (especially such that new users better understand what's going on), but then it can be merged.
I've used ideas proposed in #205 to reduce the memory allocations for AMR significantly - @gregorgassner nerd-sniped me on Slack...
For example, I get the following results for
trixi_include("examples/2d/elixir_euler_blast_wave_shockcapturing_amr.jl")
(after running it once to remove all compilation overhead).
On
dev
, AMR uses5.34GiB
and takes2.46s
.In this PR, AMR uses
1.25GiB
and takes1.84s
.I think there are still a some possibilities to reduce allocations and speed up AMR, cf. #161.