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

cover tree could rearrange descendants in memory for faster accesses #275

Closed
rcurtin opened this issue Dec 29, 2014 · 3 comments
Closed

Comments

@rcurtin
Copy link
Member

rcurtin commented Dec 29, 2014

Reported by rcurtin on 14 Dec 43355870 02:33 UTC
Right now, the dataset which is given to the cover tree when it is built does not move any points around in memory; it simply holds references to the points in the tree.

In situations where the descendants are looped over (with NumDescendants() and Descendant()), this is far from optimal and could be much faster if the points had been arranged in memory such that Descendant(i) just returns i plus some constant (the point held by that node).

This would take some refactoring of the cover tree class but should provide decent speedup.

Migrated-From: http://trac.research.cc.gatech.edu/fastlab/ticket/285

@rcurtin rcurtin self-assigned this Dec 29, 2014
@rcurtin rcurtin added this to the mlpack 1.1.0 milestone Dec 29, 2014
@rcurtin rcurtin removed their assignment Dec 30, 2014
@rcurtin rcurtin modified the milestones: mlpack 2.0.1, mlpack 2.0.0 Dec 15, 2015
@rcurtin rcurtin modified the milestones: mlpack 2.0.2, mlpack 2.0.1 Feb 4, 2016
@na1taneja2821
Copy link
Contributor

Hi,
I was going through this issue to solve it. As already stated in the issue, the node stores the references to the points. I just want to clarify, whether we can rearrange the points in the dataset itself (so that the references can be accessed by adding the constant) or whether there is a need to maintain another memory block for accessing the points?

@rcurtin
Copy link
Member Author

rcurtin commented Apr 5, 2016

Hi Naman,

Take a look at the code for BinarySpaceTree (src/mlpack/core/tree/binary_space_tree). The BinarySpaceTree, because it rearranges descendants, must make a copy of the const data matrix, or otherwise the data matrix must be passed as an rvalue reference. You can make the same modification for the cover tree.

@na1taneja2821
Copy link
Contributor

Hi Ryan,
Sorry for the late response, had been busy with the practical exams. I have gone through the code for cover tree (and I must say it is quite well written). I have made certain changes to solve the issue and currently debugging it. I will open the pull request soon.

na1taneja2821 added a commit to na1taneja2821/mlpack that referenced this issue Apr 16, 2016
@rcurtin rcurtin modified the milestones: mlpack 2.0.3, mlpack 2.0.2 Jun 21, 2016
@rcurtin rcurtin modified the milestones: mlpack 2.0.4, mlpack 2.0.3 Aug 6, 2016
@rcurtin rcurtin modified the milestones: mlpack 3.0.0, mlpack 2.0.4 Nov 1, 2016
@rcurtin rcurtin modified the milestones: mlpack 3.0.0, mlpack 3.1.0 May 1, 2018
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

3 participants