search

u1234x1234 / pynanoflann

Unofficial python wrapper to the nanoflann k-d tree
BSD 2-Clause "Simplified" License
34 stars 8 forks source link

Taking the square root of distances is now optional #10

Open hochshi opened 1 year ago

hochshi commented 1 year ago

I've added a root_dist flag that allow the distances returned from nanoflann to stay squared even when using a L2 norm. I've also added 2 tests to verify the addition of the flag the results remain consistent with sklearn.

Also bumped the version in setup.py to 0.0.9.

u1234x1234 commented 1 year ago

Hi,

Thank you for the contribution!

What do you think about making it as a different metric name not as an additional root_dist argument? I mean that root_dist does not affect L1 distance and it's not clear what to expect in cases (metric='L1', root_dist=True) or (metric='L1', root_dist=False).

My suggestion is to add a new metric L2_squared in addition to L1 and L2. And when the metric is L2_squared keep the distances squared by not taking root. So it will be something similar to:

if metric == "l2":
  dists = np.sqrt(dists)    # will be skipped with metric="l2_squared"

or

    if metric == "l2":
        g_d = [np.sqrt(x) for x in g_d]
hochshi commented 1 year ago

Sure. I'll make the changes soon. I was surprised at first that the distances were square rooted, as nanoflann's behaviour is to return squared distances.