search

yahoojapan / NGT

Nearest Neighbor Search with Neighborhood Graph and Tree for High-dimensional Data
Apache License 2.0
1.24k stars 114 forks source link

Add new NGTQG to C API #92

Closed lerouxrgd closed 3 years ago

lerouxrgd commented 3 years ago

Would it be possible to add latest NGTQG to the C API ? It would be then possible for wrappers to stay up to date.

masajiro commented 3 years ago

Thank you for your request, but give me some time.

lerouxrgd commented 3 years ago

Sure, there is no hurry, thank you very much !

masajiro commented 3 years ago

Sorry for the late response. I have pushed the branch ngtqg-capi including the ngtqg C API source codes below. Capi.h Capi.cpp Please refer an example code using the APIs in Capi.h. The ngtqg_quantize and ngtqg_search_index are almost the same as the quantize and search of the ngtqg command, respectively. If there are no problems with the C APIs, I will merge them into the master branch.

lerouxrgd commented 3 years ago

Thank you for the C API and the example, it is very useful. I managed to use the C API correctly and here are my findings:

lerouxrgd commented 3 years ago

Also on a similar topic, I noticed that ngt_set_property_distance_type_sparse_jaccard is missing in the regular C API.

masajiro commented 3 years ago

Thank you for your comments. I have released NGT v1.13.7 with NGTQG C APIs.

  • Would it be possible to have a function to retrieve an object from a quantized index ? Something like ngt_get_object_as_integer and ngt_get_object_as_float in the regular index case.

You can also use ngt_get_object_as_float for the quantized indexes like this.

  • How to check the result of ngtqg_quantize ? Would it be possible to return true/false so that we know whether we have to check the NGTError ?

I modified ngtqg_quantize() to return its status.

  • There is a typo in NGTQGQuantizationPrameters -> NGTQGQuantizationParameters

Thanks.

Also on a similar topic, I noticed that ngt_set_property_distance_type_sparse_jaccard is missing in the regular C API.

At this moment I cannot afford to deal with the sparse jaccard which is significantly different from other distances in terms of implementation.

lerouxrgd commented 3 years ago

Thank you for the fixes, everything looks good !

Just one last question, NGTQGIndex is not compatible with feature NGT_SHARED_MEMORY_ALLOCATOR ? I tried to test it and got SIGABRT.

masajiro commented 3 years ago

Thank you for reviewing. I am wondering whether I should support NGT_SHARED_MEMORY_ALLOCATOR for NGTQG, because this spoils the fastest NGTQG index of the NGT indexes.

lerouxrgd commented 3 years ago

I see, indeed it is better not to spoil performances. I found the NGT_SHARED_MEMORY_ALLOCATOR very convenient so I just wanted to check it with NGTQG Index.

lerouxrgd commented 3 years ago

Anyway as long as C API is concerned everything is fine, thank you again for adding this !