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

What does the radius parameter do? #5

Open
aluo-x opened this issue Mar 23, 2021 · 7 comments
Open

What does the radius parameter do? #5

aluo-x opened this issue Mar 23, 2021 · 7 comments

Comments

@aluo-x
Copy link

aluo-x commented Mar 23, 2021

First thanks for this great tool.
I'm a little confused by the radius parameter:
nn = pynanoflann.KDTree(n_neighbors=5, metric='L1', radius=100)

It seems like I can set this to a tiny value and still get correct results when I query.

@u1234x1234
Copy link
Owner

Hello, Andrew

Thank you for the feedback!
Could you provide an example of incorrect results?

Example of how it supposed to work:

import numpy as np
import pynanoflann

X_index = np.array([[0, 0], [100., 0.], [0., 100.], [100., 100.]])
"""
2              3
|
|
|
|
|
0--------------1
"""

nn = pynanoflann.KDTree(n_neighbors=5, metric='L1', radius=100)
nn.fit(X_index)

query = X_index[:1]  # point at (0, 0)
_, indices = nn.radius_neighbors(query, radius=0)
print(indices)  # no matches

_, indices = nn.radius_neighbors(query, radius=50)
print(indices)  # point at (0, 0)

_, indices = nn.radius_neighbors(query, radius=200-0.01)
print(indices)  # points (0, 0), (100, 0), (0, 100); distance to (100, 100) == 200

_, indices = nn.radius_neighbors(query, radius=200+0.01)
print(indices)  # all points

Output:

[]
[[0]]
[[0 1 2]]
[[0 1 2 3]]

that is, the results depends on the radius.

@aluo-x
Copy link
Author

aluo-x commented May 4, 2021

Thanks!

I guess my confusion is what the radius parameter in
nn = pynanoflann.KDTree(n_neighbors=5, metric='L1', radius=100) does.

Edit:
Also curious what the canonical behavior for _, indices = nn.radius_neighbors(query, radius=distance) is, when we cannot find up to n_neighbors for a subset of query points within a radius. It seems like in this case the function returns effectively an empty array.

Edit2:
pynanoflann seems to ignore the n_neighbors argument when performing a radius_neighbors query.

@aluo-x
Copy link
Author

aluo-x commented May 6, 2021

For the radius query, there seems to be a minor bug. The python interface is returning square rooted distance, but the radius query takes in a squared query.

@CCInc
Copy link
Contributor

CCInc commented Jun 2, 2021

For the radius query, there seems to be a minor bug. The python interface is returning square rooted distance, but the radius query takes in a squared query.

@aluo-x that's expected behaviour then, for a L2 query, nanoflann takes in a squared radius and returns a list of squared distances, so the python interface squares the provided euclidean radius as input and then takes the square root of the squared distances to return regular distances.

If you specify a radius from nn = pynanoflann.KDTree(n_neighbors=5, metric='L1', radius=100), then this distance 100 will be squared before passing to nanoflann. If, however, you specify a radius from nn.radius_neighbors(query, radius=100), then this radius won't be squared before passing to nanoflann, so you'd have to do nn.radius_neighbors(query, radius=100**2) to get expected behavior.

If that's confusing, I can submit a PR which squares the radius passed to radius_neighbors too, if @u1234x1234 thinks it's a good idea..

@aluo-x
Copy link
Author

aluo-x commented Jun 3, 2021

I'm not sure that is the expected behavior. The python interface as of right now uses euclidean distance as function input, but returns squared euclidean distances.

The KDTree construction also seems to ignore the radius argument.

Edit: I mean to say, the function takes in squared euclidean distance, but returns euclidean distance.

@CCInc
Copy link
Contributor

CCInc commented Jun 3, 2021

Yeah, I agree it is confusing that the constructor radius is the regular distance but the radius_neighbors radius requires the squared distance, it would probably be good to make those consistent.

@u1234x1234
Copy link
Owner

Sorry for the confusion, I've made a fix to this bug:
9a93218

You could try new version:

pip install git+https://github.com/u1234x1234/pynanoflann.git@0.0.9

Thank you for the feedback

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

No branches or pull requests

3 participants