u1234x1234 / pynanoflann

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

Serializing KDTrees #1

Closed HuguesTHOMAS closed 4 years ago

HuguesTHOMAS commented 4 years ago

Hi @u1234x1234,

Thanks for your awesome wrapper. I would like to use it for radius search in 3D point clouds. In my implementation, I would like to be able to save a KDTree in a text file and then reload it afterwards.

I tried to use pickle, but nanoflann_ext.KDTree32 or nanoflann_ext.KDTree64 object are not supported.

Do you think there is a way to save these objects to files? Or enable pickle to serialize them?

Best, Hugues

u1234x1234 commented 4 years ago

Hi, Hugues

Thank you for the interest. I apologize for the inconvenience. I''l look into this issue and make a kd-tree index serializable in a day or two.

u1234x1234 commented 4 years ago

@HuguesTHOMAS I've added the options to save a KDTree Please check: https://github.com/u1234x1234/pynanoflann#save-load

Installing:

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

Does this solve your problem?

HuguesTHOMAS commented 4 years ago

@u1234x1234 Thanks a lot for your reactivity.

This solves my problem indeed. I just had another question. the attribute _fit_X is private, but actually it is usefull to have access to it. It is not a problem for me because python is not restrictive and allows me to access this variable. But I just wanted to signal that it would be a better idea to have this attribute as public (something like fit_X instead of _fit_X.

Thanks again for everything.

Best, Hugues

u1234x1234 commented 4 years ago

Thanks for the feedback.

Let's look at it from a different perspective. Suppose we fitted a kd-tree, and having a public access to the X (self.fit_X), modified the data. This will violate the correctness of subsequent calls to "kneighbors" method, since any data modifications should be reflected on the built index. That's why it was private. But you are right, sometimes it is actually usefull to have an access to the underlying data (e.g after unpickling).

So I've added a method that returns underlying data (with an option whether to copy or not).

HuguesTHOMAS commented 4 years ago

Yeah, you are right. A method that return the data is good, especially if there is the option to avoid copy.

Thanks again for this high quality work.

Best, Hugues