faster insertion and a static kdtree without node hierarchy #19
Conversation
|
|
||
| // left is always included, right is always excluded, | ||
| // so the range [left,right) contains (right-left) elements. | ||
| function partition(points, left, right, pivIdx, dim) { |
There was a problem hiding this comment.
Can you give a detail documentation of this algorithm? Or provide some links about it.
There was a problem hiding this comment.
This link
provides some information about the algorithm.
There was a problem hiding this comment.
This is a very standard algorithm whose description you can find in many place. For example wikipedia : https://en.wikipedia.org/wiki/Median_of_medians#Algorithm
|
I think this is a very good improvement! 😄 |
|
ping. Is ubilabs not interested in much faster kd-tree construction? |
kdTree.js
Outdated
| const pivot = points[pivIdx][dim]; // get pivot value | ||
| var storeIdx = left; // variable to store the final position of the pivot value. | ||
| swap(points, pivIdx, right-1); // Move pivot to end | ||
| for (i=left; i < right-1; ++i) { // check all values but the last |
There was a problem hiding this comment.
| for (i=left; i < right-1; ++i) { // check all values but the last | |
| for (let i=left; i < right-1; ++i) { // check all values but the last |
Pretty sure i should be defined here.
There was a problem hiding this comment.
In fact, all of the for loops I've seen have undefined variables.
There was a problem hiding this comment.
Thank you very much @TheColorman Is it standard practice nowadays in Javascript? (the code above is the only code I have ever written in JS)
There was a problem hiding this comment.
Yeah, the ability to use variables without declaration is considered a mistake in the language that exists due to backwards compatibility afaik.
There was a problem hiding this comment.
OK, I added let in every for loop. Anyway... after all these years, I doubt Ubilabs has any interest in my patch :-)
Added selection so that one can build the tree in n log n instead of n log^2 n, as is the case when
sort()-ing at each level.selection (of pivot) uses either median-of-median-of-5, or simply use the pivot value found in the middle.
The first is theoretically better but slower in practice, so it's commented out, in favour of the second option.
With 1M points in the "basic" example, tree construction goes from 12.5 seconds to 2.5 seconds.
Also added a staticKdTree, that only allows nearest search queries. No insertion, no deletion.
The advantage is that it does not build a node hierarchy, so there is no memory allocation.
I hoped the static version would be faster too, but it doesn't seem so. Might depend on the usage scenario.
kdTree-min.js is up to date.
The files package.json has not been updated, since I don't know how to.