Conversation
Likith101
commented
Dec 11, 2025
Likith101
commented
- Added all necessary api connections for go-faiss needs
Likith101
requested review from
CascadingRadium,
Thejas-bhat,
abhinavdangeti,
capemox and
maneuvertomars
- Added all necessary api connections for go-faiss needs
Likith101
changed the title
| } | ||
|
|
||
| // Try to cast to IndexIDMap2 | ||
| if (auto idmap2 = dynamic_cast<const faiss::IndexIDMap2*>(idx)) { |
There was a problem hiding this comment.
remove this if block as we no longer use IDMap2 based Flat indexes.
| FAISS_THROW_IF_NOT(k > 0); | ||
| FAISS_THROW_IF_NOT(nprobe > 0); | ||
|
|
||
| const size_t nprobe_2 = std::min(nlist, this->nprobe); |
There was a problem hiding this comment.
nprobe_2 is the nprobe used right?
const size_t nprobe = std::min(nlist, params ? params->nprobe : this->nprobe);| std::unique_ptr<int32_t[]> coarse_dis(new int32_t[n * nprobe_2]); | ||
|
|
||
| double t0 = getmillisecs(); | ||
| quantizer->search(n, x, nprobe_2, coarse_dis.get(), idx.get()); |
There was a problem hiding this comment.
I know we are not setting the params for the quantizer, but we should add it here for future proofing
quantizer->search(n, x, nprobe_2, coarse_dis.get(), idx.get(), params ? params->quantizer_params : nullptr);| memcpy(recons, invlists->get_single_code(list_no, offset), code_size); | ||
| } | ||
|
|
||
| void IndexBinaryIVF::get_lists_for_keys( |
There was a problem hiding this comment.
legacy API must be removed
| return dispatch_HammingComputer(code_size, bs, code_size, store_pairs); | ||
| bool store_pairs, | ||
| const IDSelector* sel) const { | ||
| // Choose the appropriate HammingComputer type based on code_size |
There was a problem hiding this comment.
why is this required? i think the dispatch_HammingComputer will work right?
Like that is a template that calls IVFBinaryScannerL2 with variadic arguments which is already enabled to accept the selector in BuildScanner. Cant it be
BuildScanner bs;
return dispatch_HammingComputer(code_size, bs, code_size, store_pairs, sel);| bool store_pairs = false, | ||
| const IDSelector* sel = nullptr) const; | ||
|
|
||
| void get_lists_for_keys(idx_t* keys, size_t n_keys, idx_t* lists); |
| if (by_residual) { | ||
| // Compute residual for this list_no | ||
| std::vector<float> tmp(d); | ||
| quantizer->compute_residual(query, tmp.data(), list_no); |
There was a problem hiding this comment.
the allocation of tmp in the loop can be a perf hit, and the computation of compute_residual is only required if list_no has changed. You should split the code into
if by_residual {
// cant set query as we need to get the residual query for the distCompute
std::vector<float> residual(d);
prevListNo = nil
for i in IDs
compute listNo
if list_no not equal prevListNo
compute residual using reused `residual`
set residual in dc
compute distance
} else {
// directly set the query if non-residual mode
dc->set_query(query);
for i in IDs
compute listNo
compute distance
}2 participants
