Conversation
Likith101
requested review from
CascadingRadium,
Thejas-bhat,
abhinavdangeti,
capemox and
maneuvertomars
Likith101
changed the title
- Added all necessary api connections for go-faiss needs
abhinavdangeti
left a comment
abhinavdangeti
left a comment
There was a problem hiding this comment.
@Likith101 Let's update the commit header and the description here capturing the functionality you're proposing here for which BQ classes.
| } | ||
| CATCH_AND_HANDLE | ||
| } | ||
|
|
There was a problem hiding this comment.
what about faiss_IndexBinaryIVF_get_centroids_and_cardinality for the ObtainKCentroidCardinalitiesFromIVFIndex We should be supporting that API as well right?
| store_pairs(store_pairs) {} | ||
|
|
||
| void set_query(const uint8_t* query_vector) override { | ||
| this->query_vector = query_vector; // Set the member directly |
There was a problem hiding this comment.
what does this line do? is it required to set the query_vector of BinaryInvertedListScanner?
There was a problem hiding this comment.
follow up to this - doesn't the hammingComputer already use the query vector to compute the distance? i'm a bit confused if this a redundant var we're maintaining.
|
@Likith101 it seems there is code submitted upstream with facebookresearch#4761 (unfortunately not part of a release just yet) that has overlap with your proposal here. Can I recommend cherrypicking that commit to this PR, so we wouldn't have to deal with too may merge conflicts in the area in the future when we bring in a later version tag. |
| store_pairs(store_pairs) {} | ||
|
|
||
| void set_query(const uint8_t* query_vector) override { | ||
| this->query_vector = query_vector; // Set the member directly |
There was a problem hiding this comment.
follow up to this - doesn't the hammingComputer already use the query vector to compute the distance? i'm a bit confused if this a redundant var we're maintaining.
| using C = CMax<int32_t, idx_t>; | ||
|
|
||
| size_t nup = 0; | ||
|
|
There was a problem hiding this comment.
can you please remove these kind of empty lines?
|
|
||
| dc->set_query(tmp.data()); | ||
|
|
||
| // Compute the distance |
There was a problem hiding this comment.
another doubt at this point - i was just going through the code around hamming distance compute, and it looks like there's probably an API to compute the distance between a batch of codes? have you looked into using that instead of computing the distance every time?
Line 217 in 04afbf4
4 participants
Support for Binary indexes (IVF and Flat) with backing indexes (SQ8 and Flat)