-
Notifications
You must be signed in to change notification settings - Fork 73
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
Use fast measure(body)
in measure!(flow)
#155
Conversation
We don't need normal or velocity measurements outside a very narrow band in the solver, so `measure(fast)` can be applied there as well, but the band is variable. Changed `fast` to `fastd²=Inf` and n,V are skipped when `d^2>fastd²`.
You could locally merge #152 into this PR (no need to commit it) and do the benchmark with that :) |
for i ∈ 1:N | ||
dᵢ,nᵢ,Vᵢ = measure(body,loc(i,I,T),t) | ||
dᵢ,nᵢ,Vᵢ = measure(body,loc(i,I,T),t,fastd²=d²) |
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.
This line won't do too much, since the if statement above is already catching most of them. I tried to reduce to d=epsilon, but this broke the code.
@fastmath @inline function fill!(μ₀,μ₁,V,d,I) | ||
d[I] = sdf(body,loc(0,I,T),t) | ||
if abs(d[I])<2+ϵ | ||
d[I] = sdf(body,loc(0,I,T),t,fastd²=d²) |
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.
This is the important line for ParametricBodies.
But I need to compare with master. How would that work? Also, I'm working from home so I don't have my GPU available today. |
But yes, ideally we would like to see GPU results too. -- Or I could also merge that with master already, because I think it is finished now. |
measure(body)
in measure!(flow)measure(body)
in measure!(flow)
In fact, you do need to use that PR because otherwise the changes in |
CPU benchmarks on my machine show a slow down??!??
|
Here are my benchmarks. 1dbc2d2 is "master" (the benchmark PR) and 8d348ef is the current PR. I do not see any major performance speedup or regression.
|
Yeah, that's actually encouraging. It means the current measure function is already optimized for AutoBodies. For NURBS ParametricBodies, I measured a 100% speed up with this PR, so it's certainly worth doing. |
So can we merge this? |
We don't need normal or velocity measurements outside a very narrow band in the solver, so
measure(fast)
can be applied there as well, but the band is variable. Changedfast=false
tofastd²=Inf
and n,V are skipped whend^2>fastd²
.Applied in
measure!(flow)
and tests passing, but I haven't benchmarked yet. Should be a bit faster for the Jelly and will really speed up ParametricBody simulations.