-
Notifications
You must be signed in to change notification settings - Fork 114
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
Surface stuff is slow for the P4estMesh
#642
Comments
Have you compared the numbers to the same setup with the |
I added the structured mesh above. I don't have a mesh file to check the unstructured mesh right now |
Now we have |
I added another example for the Euler equations. That's less dramatic but we should still have a look at it, I think. However, it doesn't have top priority, of course. |
Please don't get me wrong: Thanks a lot for reporting this, and I do think we should investigate! With my comment above I was not trying to dismiss this issue, but rather add to the discussion about potential reasons. |
Appreciated 👍 Maybe it's just related to the possibility of unstructured interfaces and we need to find a better way to improve the performance of that part. |
A first profiling of the Euler stuff shows that a lot of time is spent in Trixi.jl/src/solvers/dg_p4est/dg_2d.jl Lines 32 to 40 in 45391a2
again and again is very inefficient. Instead, the correct loop indices should be determined once before these inner loops run. |
Yes, this seems like a likely candidate for optimization. However, IMHO we should not touch this until we have support for 3D unstructured in Trixi, on which @efaulhaber is currently working. Once that's finished, we can optimize it directly on the 3D code and then just du the same for 2d to make it easy to transition between those two. |
I was thinking if there's a way to implement something similar to Another idea: It's possible to create a julia> A = [1 2; 3 4]
2×2 Matrix{Int64}:
1 2
3 4
julia> view(A, 2:-1:1, :)
2×2 view(::Matrix{Int64}, 2:-1:1, :) with eltype Int64:
3 4
1 2 I haven't tested yet if this would be any more efficient, but it could be worth a try. |
Btw, where do all these allocations come from? I get this when running your first example:
Edit: This was with
|
Looks like I was running it on a computer with |
Sure. As I said above, I opened this issue to record this observation @efaulhaber made in the PR linked above and to prepare some information that can be used to improve the solver based on the
|
The problem with such an approach is that we would get some type instabilities since the different |
This came up in #638 (comment). For example, I get
Another example:
(running the code twice to exclude compilation times). It's okay that the volume integral is a bit slower since it needs to handle curvilinear stuff. However, everything related to the surfaces is also much slower. We should investigate this and try to fix it (if reasonably possible).
Related to #628 (but more general).
TODO
TODO: p4est interface performance
(p4est surface performance 3D #783)The text was updated successfully, but these errors were encountered: