Skip to content

Implementing the kd_nearest_n function #18

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

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

Conversation

fbriol
Copy link

@fbriol fbriol commented Dec 19, 2016

Hello, I needed to implement the function kd_nearest_n. It's up to you to see if it interests and suits you. Sincerely, F. Briol.

@jtsiomb
Copy link
Owner

jtsiomb commented Dec 21, 2016

Hi,
Is it done? Could you send me an email with a short description of what you did, any references to the algorithm details etc? (nuclear@member.fsf.org).

I'll take a look at it as soon as possible.

@ghost
Copy link

ghost commented Dec 21, 2016

The PR introduces a lot of style changes (in my view those would be best in their own PR), consequently the diffs are rather noisy. So could I suggest using GitHub's "whitespace change suppression" feature, just add ?w=1 to the URL, as here.

@fbriol
Copy link
Author

fbriol commented Jan 2, 2017

Hello,

Sorry to respond to you so late, but I was AFK during my winter break.

The computation algorithm consists of managing a list of N nearest neighbors. When calculating a new distance during the descent of the tree, the new node is integrated when the distance found is less than the maximum distance stored on the list.

I also added a function to remove all the memory used during the calculation. Perhaps it would be desirable to add a new function performing this task so as not to change the previous behavior of your API.

To format the code, I used "clang-format" to standardize the formatting of the code. I can just introduce the changes without using this tool. But from my point of view, it's very useful to use such a tool, because you can sure that all contributions pushed will use the same formatting.

@xJeffos
Copy link

xJeffos commented Aug 4, 2021

Hello !

Is there a plan to integrate this work ? @fbriol branch works quite well for me.

@jtsiomb
Copy link
Owner

jtsiomb commented Aug 4, 2021

Good to hear it works, but this was never merged because it's a terrible pull request. The OP used a code formatting tool which introduced style changes everywhere. Someone needs to clean this up and rebase it, to make a clean changeset out of this, reflecting only the true additions to the code, before it can be merged.

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.

3 participants