Skip to content
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

Barnes hut algorithm is way too slow #6

Closed
adityapb opened this issue Apr 2, 2016 · 7 comments
Closed

Barnes hut algorithm is way too slow #6

adityapb opened this issue Apr 2, 2016 · 7 comments

Comments

@adityapb
Copy link
Owner

adityapb commented Apr 2, 2016

Timings for the following benchmark,

x, y, z = numpy.mgrid[0:500:25j, 0:500:25j, 0:500:25j]
x = x.ravel(); y = y.ravel(); z = z.ravel()
Algorithm Time
Barnes hut 44.227s
Brute Force 23.454s

which is way too slow. We could probably speed it up by avoiding copying the entire dataset to a list.
We could instead try to store the indices in a list and access the properties using the pointers itself.
Also passing 3 values by reference to the functions returning Vec3 could also help a bit.
This might hurt readability a bit, but once we get it working, I think we should make these optimizations
before the release.

@rahulgovind
Copy link
Collaborator

Did you change the value of theta? Because when theta = 0, Barnes' hut has the same time complexity as the brute force algorithm.

@adityapb
Copy link
Owner Author

adityapb commented Apr 2, 2016

I simply used the barnes_square_grid example with large number of particles. The complexity is the same but there's a lot of data copying, thats what I think is making it slower.

@rahulgovind
Copy link
Collaborator

I don't really think data copying is the culprit here and a huge amount of time can be saved by simply changing the isChild function. I'll benchmark it with the changes and see if it does make a difference though.

IMO, the benchmark is not that bad even right now. Because -

  1. It has to first build the octree
  2. Unlike the brute force algorithm, it has to traverse the octree for calculating the forces instead of traversing an array.

@adityapb
Copy link
Owner Author

adityapb commented Apr 2, 2016

You're right. It takes only about 0.014 secs out of 45 for copying data to list.

@adityapb
Copy link
Owner Author

adityapb commented Apr 2, 2016

BTW, you should probably change isChild to is_child

@rahulgovind
Copy link
Collaborator

Sure thing.

@adityapb
Copy link
Owner Author

adityapb commented Apr 3, 2016

Also rename Vec3 to Vector maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants