Skip to content

Commit da3191e

Browse files
Lengning Liumeta-codesync[bot]
authored andcommitted
IDSelectorWithContext: extend context hook to IVFPQ/PQR, multibit RaBitQ, and Panorama scanners (facebookresearch#5491)
Summary: Pull Request resolved: facebookresearch#5491 D113139231 added `faiss::IDSelectorWithContext` and wired exactly one scan loop (`run_scan_codes1`, used by IVFFlat / IVFSQ). This diff extends the same "hand the selector its scan context" hook to the remaining linear IVF per-candidate selector sites, so `IDSelectorWithContext` is honored uniformly instead of being silently dropped depending on index type or a runtime parameter (`nb_bits`). A new shared helper `IDSelectorContextDispatch` (in `IDSelector.h`) centralizes the once-per-list `dynamic_cast` + per-candidate dispatch. `run_scan_codes1` is refactored onto it, and the new sites reuse it: - `IVFPQ` / `IVFPQR`: `WrappedSearchResult::skip_entry` — one edit covers the table / pointer / on-the-fly / polysemous variants, and IVFPQR reuses the IVFPQ scanner. - multibit `RaBitQ` (`nb_bits >= 2`): the two hand-written `scan_codes_multibit` loops (scalar + SIMD). This closes a real footgun where the hook was honored at 1 bit (via `run_scan_codes1`) but silently dropped at `nb_bits >= 2` within the same index type. - `IVFFlatPanorama`: `progressive_filter_batch` — the scan-order position is the full-list `global_idx`, so a lookahead crosses batch boundaries correctly. No behavior change: `is_member_with_context` returns a verdict identical to `is_member`; the context is a prefetch-only side channel. Selectors that do not implement the sub-interface, and `store_pairs` scans (synthetic ids), fall back to `is_member`. The base `IDSelector` is untouched, so every existing selector is unaffected. Binary size: one small inline struct plus a cached pointer; no new template dimensions. FastScan (`IVFPQFastScan` / `IVFRaBitQFastScan`) is intentionally deferred to a benchmark-gated follow-up: its result-lane scan is sparse and it is LUT/SIMD-bound, so the prefetch benefit there is speculative (correctness holds either way). Out of scope: `IVFSpectralHash` (asserts no selector), binary IVF, and HNSW (separate selector paths). Stacks on D113139231. Reviewed By: mnorris11 Differential Revision: D113913501 fbshipit-source-id: 011f52015f3e87b5b7cf91291edbe020bedadbeb
1 parent edddbc9 commit da3191e

6 files changed

Lines changed: 126 additions & 40 deletions

File tree

faiss/impl/IDSelector.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,30 @@ struct IDSelectorWithContext : IDSelector {
4343
const = 0;
4444
};
4545

46+
/** Routes each per-candidate membership test to is_member_with_context() when
47+
* the selector implements IDSelectorWithContext, else to plain is_member().
48+
* Construct one per inverted-list scan: the dynamic_cast is the only RTTI cost
49+
* and the per-candidate cost is a single predicted branch (cf. the
50+
* IDSelectorRange dynamic_cast in IndexIVF.cpp). The scan context is only
51+
* meaningful when the scan exposes a real id array (i.e. !store_pairs); when
52+
* store_pairs the context path is disabled and every test falls back to
53+
* is_member. */
54+
struct IDSelectorContextDispatch {
55+
const IDSelector* sel;
56+
const IDSelectorWithContext* ctx_sel;
57+
58+
IDSelectorContextDispatch(const IDSelector* sel, bool store_pairs)
59+
: sel(sel),
60+
ctx_sel((sel != nullptr && !store_pairs)
61+
? dynamic_cast<const IDSelectorWithContext*>(sel)
62+
: nullptr) {}
63+
64+
bool is_member(idx_t id, const IDScanContext& ctx) const {
65+
return ctx_sel ? ctx_sel->is_member_with_context(id, ctx)
66+
: sel->is_member(id);
67+
}
68+
};
69+
4670
/** ids between [imin, imax) */
4771
struct IDSelectorRange : IDSelector {
4872
idx_t imin, imax;

faiss/impl/Panorama.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,12 +323,20 @@ struct Panorama {
323323
size_t batch_offset = batch_no * batch_size * code_size;
324324
const uint8_t* storage_base = codes_base + batch_offset;
325325

326+
// Honor IDSelectorWithContext: the scan-order position within the whole
327+
// list is global_idx, so a lookahead crosses batch boundaries correctly
328+
// (ids is the full list, length list_size).
329+
IDSelectorContextDispatch sel_dispatch(
330+
sel, /*store_pairs=*/ids == nullptr);
331+
326332
// Initialize active set with ID-filtered vectors.
327333
size_t num_active = 0;
328334
for (size_t i = 0; i < curr_batch_size; i++) {
329335
size_t global_idx = batch_start + i;
330336
idx_t id = (ids == nullptr) ? global_idx : ids[global_idx];
331-
bool include = !use_sel || sel->is_member(id);
337+
bool include = !use_sel ||
338+
sel_dispatch.is_member(
339+
id, IDScanContext{ids, list_size, global_idx});
332340

333341
active_indices[num_active] = i;
334342
float cum_sum = batch_cum_sums[i];

faiss/impl/RaBitQuantizer.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,11 +390,17 @@ struct RaBitQDistanceComputerNotQ final : RaBitQDistanceComputer {
390390
const size_t ex_bits = nb_bits - 1;
391391
FAISS_ASSERT(ex_bits > 0);
392392

393+
// Honor IDSelectorWithContext on the multibit path too, so a RaBitQ
394+
// index does not silently lose the context hook once nb_bits >= 2 (the
395+
// 1-bit path already routes through run_scan_codes1).
396+
const IDSelectorContextDispatch sel_dispatch(sel, store_pairs);
397+
393398
size_t nup = 0;
394399
for (size_t j = 0; j < list_size; j++) {
395400
if (sel != nullptr) {
396401
idx_t id = store_pairs ? lo_build(list_no, j) : ids[j];
397-
if (!sel->is_member(id)) {
402+
if (!sel_dispatch.is_member(
403+
id, IDScanContext{ids, list_size, j})) {
398404
codes += code_size;
399405
continue;
400406
}
@@ -601,11 +607,17 @@ struct RaBitQDistanceComputerQ final : RaBitQDistanceComputer {
601607
const size_t ex_bits = nb_bits - 1;
602608
FAISS_ASSERT(ex_bits > 0);
603609

610+
// Honor IDSelectorWithContext on the multibit path too, so a RaBitQ
611+
// index does not silently lose the context hook once nb_bits >= 2 (the
612+
// 1-bit path already routes through run_scan_codes1).
613+
const IDSelectorContextDispatch sel_dispatch(sel, store_pairs);
614+
604615
size_t nup = 0;
605616
for (size_t j = 0; j < list_size; j++) {
606617
if (sel != nullptr) {
607618
idx_t id = store_pairs ? lo_build(list_no, j) : ids[j];
608-
if (!sel->is_member(id)) {
619+
if (!sel_dispatch.is_member(
620+
id, IDScanContext{ids, list_size, j})) {
609621
codes += code_size;
610622
continue;
611623
}

faiss/impl/expanded_scanners.h

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,15 @@ size_t run_scan_codes1(
3636
size_t code_size = scanner.code_size;
3737
const IDSelector* sel = scanner.sel;
3838
// If the selector implements IDSelectorWithContext, hand it the scan
39-
// context (ids, list_size, j) so it can exploit scan-order locality. Detect
40-
// it once per list via RTTI (cf. the IDSelectorRange dynamic_cast in
41-
// IndexIVF.cpp) so the per-candidate cost is a single predicted branch.
42-
// store_pairs never coexists with a selector, and ids[] is only valid when
43-
// !store_pairs.
44-
const IDSelectorWithContext* ctx_sel = (use_sel && !store_pairs)
45-
? dynamic_cast<const IDSelectorWithContext*>(sel)
46-
: nullptr;
39+
// context (ids, list_size, j) so it can exploit scan-order locality; the
40+
// dispatch caches the once-per-list RTTI detection.
41+
const IDSelectorContextDispatch sel_dispatch(sel, store_pairs);
4742
float threshold = handler.threshold;
4843
for (size_t j = 0; j < list_size; j++) {
4944
if (use_sel) {
5045
int64_t id = store_pairs ? lo_build(list_no, j) : ids[j];
5146
// skip code without computing distance
52-
const bool member = ctx_sel
53-
? ctx_sel->is_member_with_context(
54-
id, IDScanContext{ids, list_size, j})
55-
: sel->is_member(id);
56-
if (!member) {
47+
if (!sel_dispatch.is_member(id, IDScanContext{ids, list_size, j})) {
5748
codes += code_size;
5849
continue;
5950
}

faiss/impl/pq_code_distance/IVFPQScanner_impl.h

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,31 @@ struct WrappedSearchResult {
2929
ResultHandler& res;
3030
size_t nup = 0;
3131
idx_t list_no;
32-
32+
size_t list_size;
3333
const idx_t* ids;
3434
const IDSelector* sel;
35+
IDSelectorContextDispatch dispatch;
3536

3637
WrappedSearchResult(
3738
idx_t list_no_in,
39+
size_t list_size_in,
3840
const idx_t* ids_in,
3941
const IDSelector* sel_in,
4042
ResultHandler& res_in)
41-
: res(res_in), list_no(list_no_in), ids(ids_in), sel(sel_in) {}
43+
: res(res_in),
44+
list_no(list_no_in),
45+
list_size(list_size_in),
46+
ids(ids_in),
47+
sel(sel_in),
48+
// A selector implies real ids, so ids==nullptr iff store_pairs;
49+
// that disables the context path exactly when ids[] is synthetic.
50+
dispatch(sel_in, /*store_pairs=*/ids_in == nullptr) {}
4251

4352
inline bool skip_entry(idx_t j) {
44-
return use_sel && !sel->is_member(ids[j]);
53+
return use_sel &&
54+
!dispatch.is_member(
55+
ids[j],
56+
IDScanContext{ids, list_size, static_cast<size_t>(j)});
4557
}
4658

4759
inline void add(idx_t j, float dis) {
@@ -474,6 +486,7 @@ struct IVFPQScanner : IVFPQScannerT<idx_t, METRIC_TYPE, PQCodeDist>,
474486
ResultHandler& handler) const override {
475487
WrappedSearchResult<C, use_sel> res(
476488
this->key,
489+
ncode,
477490
this->store_pairs ? nullptr : ids,
478491
this->sel,
479492
handler);

tests/test_params_override.cpp

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
#include <faiss/AutoTune.h>
1818
#include <faiss/IVFlib.h>
1919
#include <faiss/IndexBinaryIVF.h>
20+
#include <faiss/IndexFlat.h>
2021
#include <faiss/IndexIVF.h>
22+
#include <faiss/IndexIVFRaBitQ.h>
2123
#include <faiss/clone_index.h>
2224
#include <faiss/impl/AuxIndexStructures.h>
2325
#include <faiss/impl/IDSelector.h>
@@ -186,18 +188,21 @@ int test_defaults_preserve_baseline(const char* index_key, MetricType metric) {
186188
}
187189

188190
// IDSelectorWithContext that delegates the verdict to an inner IDSelector while
189-
// exercising the context parameters — used to prove the scan's
190-
// is_member_with_context path returns results identical to the plain is_member
191-
// path.
191+
// exercising the context parameters. Used to prove that a scanner (a) returns
192+
// results identical to the plain is_member path and (b) actually routes through
193+
// is_member_with_context — n_context_calls counts how often the context path
194+
// fired, so a scanner that silently dropped the hook is detectable.
192195
struct ContextSelector : IDSelectorWithContext {
193196
const IDSelector& inner;
197+
mutable size_t n_context_calls = 0;
194198
explicit ContextSelector(const IDSelector& inner) : inner(inner) {}
195199
bool is_member(idx_t id) const override {
196200
return inner.is_member(id);
197201
}
198202
bool is_member_with_context(idx_t id, const IDScanContext& ctx)
199203
const override {
200204
(void)ctx;
205+
++n_context_calls;
201206
return inner.is_member(id);
202207
}
203208
};
@@ -242,24 +247,22 @@ int test_selector(const char* index_key) {
242247
return 0;
243248
}
244249

245-
// Same membership set as test_selector, but driven through an
246-
// IDSelectorWithContext to prove the context scan path is functionally
247-
// identical to the plain IDSelector path.
248-
int test_selector_with_context(const char* index_key) {
249-
std::vector<float> xb = make_data(nb);
250+
// Runs the same membership set (every id where i % 10 == 2) through a plain
251+
// IDSelectorBatch and through a ContextSelector wrapping it, on an
252+
// already-trained index. Returns:
253+
// 0 if the context path fired AND labels are byte-identical to the plain
254+
// path, 1 if the results diverged (a correctness regression), 2 if the
255+
// scanner never routed through is_member_with_context (the hook was
256+
// silently dropped for this index type).
257+
int check_ctx_matches_plain(Index& index) {
250258
std::vector<float> xq = make_data(nq);
251-
ParameterSpace ps;
252259

253260
std::vector<idx_t> kept;
254261
for (size_t i = 0; i < nb; i++) {
255262
if (i % 10 == 2) {
256263
kept.push_back(i);
257264
}
258265
}
259-
260-
auto index = make_index(index_key, METRIC_L2, xb);
261-
ps.set_index_parameter(index.get(), "nprobe", 3);
262-
263266
IDSelectorBatch batch(kept.size(), kept.data());
264267

265268
// Plain IDSelector path.
@@ -268,24 +271,50 @@ int test_selector_with_context(const char* index_key) {
268271
plain_params.nprobe = 3;
269272
plain_params.sel = &batch;
270273
auto plain_result =
271-
search_index_with_params(index.get(), xq.data(), &plain_params);
274+
search_index_with_params(&index, xq.data(), &plain_params);
272275

273276
// IDSelectorWithContext path over the same membership set.
274277
ContextSelector ctx_sel(batch);
275278
IVFSearchParameters ctx_params;
276279
ctx_params.max_codes = 0;
277280
ctx_params.nprobe = 3;
278281
ctx_params.sel = &ctx_sel;
279-
auto ctx_result =
280-
search_index_with_params(index.get(), xq.data(), &ctx_params);
282+
auto ctx_result = search_index_with_params(&index, xq.data(), &ctx_params);
281283

282284
if (plain_result != ctx_result) {
283285
return 1;
284286
}
285-
287+
if (ctx_sel.n_context_calls == 0) {
288+
return 2;
289+
}
286290
return 0;
287291
}
288292

293+
// Same membership set as test_selector, but driven through an
294+
// IDSelectorWithContext to prove the context scan path is functionally
295+
// identical to — and actually exercised by — the given index type.
296+
int test_selector_with_context(const char* index_key) {
297+
std::vector<float> xb = make_data(nb);
298+
auto index = make_index(index_key, METRIC_L2, xb);
299+
// nprobe is set explicitly in the IVFSearchParameters inside
300+
// check_ctx_matches_plain, so no need to set it on the index here.
301+
return check_ctx_matches_plain(*index);
302+
}
303+
304+
// Multibit RaBitQ (nb_bits >= 2) scans through the two hand-written loops in
305+
// RaBitQuantizer.cpp rather than run_scan_codes1, so it needs the context hook
306+
// wired separately. nb_bits=3 => ex_bits=2 => scan_codes_multibit; the default
307+
// qb keeps a real quantized-query distance computer (no 1-bit fallback).
308+
int test_selector_with_context_rabitq_multibit() {
309+
std::vector<float> xb = make_data(nb);
310+
IndexFlatL2 quantizer(d);
311+
IndexIVFRaBitQ index(
312+
&quantizer, d, 32, METRIC_L2, /*own_invlists=*/true, /*nb_bits=*/3);
313+
index.train(nb, xb.data());
314+
index.add(nb, xb.data());
315+
return check_ctx_matches_plain(index);
316+
}
317+
289318
} // namespace
290319

291320
/*************************************************************
@@ -362,10 +391,11 @@ TEST(TSEL, IVFFSQ) {
362391
EXPECT_EQ(err, 0);
363392
}
364393

365-
// IDSelectorWithContext must be functionally identical to a plain IDSelector.
366-
// IVFFlat and IVFSQ route through run_scan_codes1 (which dispatches to
367-
// is_member_with_context); IVFPQ uses its own scanner (plain is_member) — all
368-
// must return identical results.
394+
// An IDSelectorWithContext must be functionally identical to a plain
395+
// IDSelector AND actually exercised (results must not silently fall back to
396+
// is_member) on every applicable IVF scanner. IVFFlat/IVFSQ route through
397+
// run_scan_codes1; IVFPQ through WrappedSearchResult::skip_entry; multibit
398+
// RaBitQ through its two hand-written scan loops.
369399
TEST(TSELCtx, IVFFlat) {
370400
EXPECT_EQ(test_selector_with_context("PCA16,IVF32,Flat"), 0);
371401
}
@@ -378,6 +408,14 @@ TEST(TSELCtx, IVFFSQ) {
378408
EXPECT_EQ(test_selector_with_context("PCA16,IVF32,SQ8"), 0);
379409
}
380410

411+
TEST(TSELCtx, RaBitQMultibit) {
412+
EXPECT_EQ(test_selector_with_context_rabitq_multibit(), 0);
413+
}
414+
415+
TEST(TSELCtx, Panorama) {
416+
EXPECT_EQ(test_selector_with_context("IVF32,FlatPanorama"), 0);
417+
}
418+
381419
/*************************************************************
382420
* Same for binary indexes
383421
*************************************************************/

0 commit comments

Comments
 (0)