-
Notifications
You must be signed in to change notification settings - Fork 43
Update k-nearest-neighbors algorithm and added example from @crhallberg #59
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
Update k-nearest-neighbors algorithm and added example from @crhallberg #59
Conversation
Oops, sorry for the typo in your name 😅 |
Hmm... thinking about this I can improve it a bit more. Just trying to get this working now. :D |
OK 💥 I've fixed it. Just putting the update in now. 👍🏻 @crhallberg just wait. You'll love it 😄 |
Latest update of the kNearest algorithm. It is now properly depth first and goes to the quad nearest the search point first and works outwards from there. I was thinking about your comment @crhallberg about it not being depth first and couldn't get my head around it for a while but woke up this morning with new ideas which worked perfectly 👍🏻 Screen.Recording.2021-03-19.at.10.08.21.movIn the previous iteration I was searching the points of a tree before diving into the children. The main change for this now is that it goes to the children first. Only after searching the children does it then start search the parents. And only then if they could contain points closer than those already found. Much better implementation of this now. It's amazing what sleeping on a problem can do. 😄 |
This works even better when rebalancing the tree too. (Although we can keep that for later). On average when the tree is rebalanced. With 1000 points it will search about 10 points to find the closet one. |
Spoiler alert... rebalancing the tree makes the Screen.Recording.2021-03-19.at.17.23.45.mov😃 |
@crhallberg hey, this should be good to go in now unless there is anything you'd like me to update. Sorry for the messed up first version. I'm still learning myself so this has been a good challenge. 😆 |
Hey! Everything does look good. I just wanted to run a few more manual tests with finding multiple closest points but haven't found the time. Thanks for the quick fix on this! |
Cool! Happy to answer any questions. The structure of the tree causes some points to be tested that you would think would not be tested. And with multiple closest it highlights that even more. However, the rebalance PR that I have in the works addresses this. With the rebalance the closest algorithm is much much better. (Without having to change it) |
I previously removed the sort of the points before iterating over them. This isn't ideal as it means the If the closest point is iterated first then fine. But if the furthest point goes first then it will be the furthest distance. So even though we've found the closest point and no other rects are closer it's actually using the furthest point in that rect as the Because of this I have now put back the sort of points before iterating along them. The sketch example is unchanged as I didn't remove it from there. |
I might add a few comments later but this all looks good! Thanks for the fix! |
Great thanks 😊 |
I def agree that there is some improvement to be made. I've got another branch where I rebalance the tree. I'll show you that one shortly. It massively improves the search but there's still something not quite perfect with it. I need to spend more time with it. It's def good enough now but can be improved 👍🏻 |
As pointed out by @crhallberg the algorithm I had implemented was not as optimised as it could be and was searching too many points when displayed in the example he wrote.
I have now improved the algorithm and it cuts down how many quads and points are searched by a lot.
The screenshot shown here is searching for the single closest point with no distance limit...
Screen.Recording.2021-03-19.at.10.08.21.mov
It is still not ideal but that is due to the fact that the tree is not "balanced". Ideally, all the points of the quad tree should be contained in the leaves of the tree (i.e. the very bottom sub trees) with no points left higher up the tree. This allows the k-nearest-neighbors search to eliminate entire subsections of the quad tree just by the frame they have and only then start searching for points.
Perhaps this is something we could re-address. There are some nice PRs open that address this and could be good to merge in with some fixes.