-
Notifications
You must be signed in to change notification settings - Fork 78
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
Fix n-bodies stack-overflow error with new type Bodies #95
Conversation
New AutoBodies type is implemented. It holds a vector of type AutoBody and implements superposition of sdf and map functions by iteration instead of recursion. ∙ Tests show same results as the recursive implementation of the superposition, while n-bodies can be handled now without hitting the stackoverflow error. ∙ Also, the measure function is much faster than before since the iterative method scales linearly, while the recursive method seems to scale exponentially.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #95 +/- ##
==========================================
- Coverage 96.55% 96.12% -0.44%
==========================================
Files 12 12
Lines 465 490 +25
==========================================
+ Hits 449 471 +22
- Misses 16 19 +3 ☔ View full report in Codecov by Sentry. |
…llowed between Bodies. Updated docs accordingly.
Sadly I cannot test on MacOS, so I am not sure. But the fix on that test is because it fails in Julia 1.10.0 for any OS. Other than this, the PR is mostly finished. The main new features of this PR is that force = WaterLily.∮nds(sim.flow.p,sim.flow.f,sim.body.bodies[i],t) |
Ready for review. |
Should we merge this or just close it? @weymouth |
We should merge it. |
New
AutoBodies
type is implemented that can solve the n-bodies problem reported in #88. It holds a vector of typeAutoBody
and implements superposition ofsdf
andmap
functions by iteration instead of recursion.Tests show same results as the recursive implementation of the superposition, while n-bodies can be handled now without hitting the stackoverflow error. Also, the measure function is much faster than before when superposing more than 5 bodies. The cost of the iterative method does not really scale when adding more bodies, while the recursive method seems to scale exponentially.
Here are some test when adding number of bodies
n=6
andn=11
(max. I can handle in my computer with recursion)n=6
n=11
I do not particularly like that there is currently
AutoBody
andAutoBodies
, but I did not want to get rid/break the previous implementation. Maybe this can be merged in a nicer way though.