-
Notifications
You must be signed in to change notification settings - Fork 93
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
Revise KDTree implementation to store coordinates in dense arrays #333
Conversation
Just tested this using BigStitcher's interest point-based registration, works! |
check this out @minnerbe ... we can make the math how many points we can maximally support |
You should be able to use 2^31-8 points (2,147,483,640). |
Would it be too hard to back it with a RandomAccessibleInterval instead? |
Lets talk about it when you have used it for 2^31-8 points |
Looks great! The number of points that can be stored depends on the storage layout, for
Do you mean positions, values, or both? This doesn't look too hard, though, since there are dedicated wrappers for accessing both of those data structures. Also, I noticed two things while glancing over the code:
|
Yes, improvements are with FLAT layout. (You can experiment for yourself a bit with KDTreeBenchmark. The options for forcing NESTED layout are there for building KDTreeData somehwere, but not pulled all the way through to KDTree constructor, but it shouldn't be too hard to tweak.) I did benchmarks for I found this in some old notes. I think it could be branch
But I don't remember what state this was (I would be surprised if the difference is still that much).
Yes, it should be easy to do. I took as the subtext form @StephanPreibisch comment: "... because then we could use CellImg and have as many points as we want". And then the subtext to my reply would be "... because, try to build a tree with 2G points and see how long that takes, because it may take a prohibitively long time." For using >2G points besides backing the positions with a CellImg, we would need to change My preferred solution for >2G points would be a combination oct-tree with kd-trees in the leafs.
I think this split makes sense.
Yes, good point |
Thanks for the (very) detailed answer! I run the benchmarks locally with 1M points:
You're absolutely right, the difference is not as stark anymore. However, it's still worth noting that the nested layout (almost) doubles the memory consumption in 3D (38 = 24 bytes vs 4 or 8 (reference on system with <32G / >32G, respectively) + 16 (array) + 38 = 44 or 48 bytes per coordinate).
Thanks, that was the part that I overlooked! In my head, backing the positions with a CellImg didn't infer with the existing implementations.
I do see your point. Still, my feeling is that a split into an object holding the positions and one holding the values could also be beneficial, especially when adding a backing option for positions and/or values. Right now, the number of constructors of |
Yes, cool! That looks as I would have expected. Which Java version did you benchmark it with?
I think you have the nesting order wrong. for 1000 3D points you would have a
That is a good point. Splitting into into an object holding the positions and one holding the values makes sense. Probably having If you want to play around with splitting up |
Azul Zulu JDK 17.
Jep, my bad. So, the memory overhead is negligible and the runtime is also only slightly worse. That's great!
The "correct" thing (modeling-wise) would probably be to eagerly compute the bounding box and pass it to the constructor. But from a performance perspective, this is strictly worse than the current lazy computation, which is definitely "pragmatically okay". Also,
Sounds good, I'll give it a try in the next few days. |
for consistency, because the other NeighborSearch interfaces have copy() methods
I incorporated changes from #334 |
This PR revises the implementation of KDTree.
The old implementation created
KDTreeNode
objects for each point, with references to children to represent the tree structure.The new implementation does not store explicit links. Instead, coordinates of all points are stored in a flat
double[]
array(or alternatively one
double[]
array for each dimension). The tree structure is represented purely by the order of points in the array.The nodes in the tree are arranged in Eytzinger layout (children of
i
are at2i
and2i+1
). Additionally, pivot indices are chosen such that "leaf layers" are filled from the left.For example 10 nodes will always be arranged like this:
never like this:
By choosing pivots in this way, the tree structure is fully determined. For every node index, the child indices can be calculated without dependent reads. And iff the calculated child index is less than the number of nodes, the child exists.
Less memory overhead
This has considerably less memory overhead: For example we store 1,000,000 3D
RealPoint
s into a KDTree.The list of points (ArrayList object and the RealPoint objects) requires ~82MB of memory.
The old KDTree added ~90MB on top of that, just for storing the tree structure.
Despite copying the point coordinates, the new KDTree only adds ~22MB (which is basically just the
double[3_000_000]
array required for storing the coordinates.)Better performance
Performance of building trees is 2x better than the old implementation. Neighbor searches are slightly faster.
By using a different kthElement implementation, this PR also resolves #326. (building the tree is fast, also in the case that many points have identical coordinates in a particular dimension).
Easy Serialization
Note that these
KDTree
s can be serialized very efficiently (for example into N5), by just storing/loading the flattened coordinate array as is (avoiding the overhead of serializing the object structure of the interlinkedKDTreeNode
s).