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

Performance improvements #72

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

tavianator
Copy link

No description provided.

@tavianator
Copy link
Author

I added some uses of C++11 that are breaking Travis. Can it be changed to pass -std=c++11, or do we need to maintain C++03 compatibility? I can also update the README and perhaps add a Makefile if you want.

@tavianator
Copy link
Author

This post goes into some detail about the benefits/motivations of each patch: https://www.microsoft.com/en-us/research/blog/optimizing-barnes-hut-t-sne/

@lvdmaaten
Copy link
Owner

Thanks for submitting this and writing up your findings!

I think switching Travis to -std=c++11 should be fine.

For the vantage-tree point implementation, I have been meaning to switch to an approach based on product quantization for quite some time now, but I haven't had time to work on that yet.

I will do some local testing, and merge presuming the maps produced by the code still look as expected.

@tavianator
Copy link
Author

Sounds great, thanks! I eyeballed the results on my test datasets before and after this series, and the differences were only in the least-significant couple of digits. I imagine they only differ at all due to the change in how I'm computing the centers of mass. If anything the new way should be slightly more accurate as it's not accumulating errors on every *(n-1)/n when new points are added.

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

Successfully merging this pull request may close these issues.

2 participants