Skip to content

Commit 2832ac3

Browse files
tandedemeta-codesync[bot]
authored andcommitted
Fix empty-index BLAS search results (#5528)
Summary: - initialize result handlers when an exhaustive search has no database vectors - process empty results in the configured query block size, preserving bounded temporary memory for large query batches - cover L2 and inner-product search across top-1, heap, and reservoir result handlers ## Problem Searching an empty `IndexFlat` produced correct sentinel values for small query batches, but could leave the caller's distance and label buffers untouched once `nq * d` selected the BLAS path. The BLAS helpers return early for `ny == 0` before the result handler's `begin_multiple` / `end_multiple` lifecycle initializes and finalizes the output. This change handles an empty database before choosing the sequential or BLAS implementation. It uses the existing result-handler lifecycle so each handler supplies its own correct neutral distance and `-1` label semantics. Fixes #3830. ## Validation - `python -m unittest test_index.TestIndexFlat_ARM_NEON.test_empty_index_with_blas` - `python -m unittest test_index` (49 tests passed) - `faiss_test --gtest_brief=1` (290 passed, 1 platform-specific skip) - `clang-format 21 --dry-run --Werror faiss/utils/distances.cpp` ## Verification of the regression test We confirmed `test_empty_index_with_blas` is a true regression test by running it with and without the `utils/distances.cpp` change: | State | Result | |-------|--------| | Fix applied | 1 passed, 0 failed | | `utils/distances.cpp` reverted to parent, test retained | 6 failed | The 6 failures are every subtest (2 metrics x 3 values of `k`), covering the `Top1`, `Heap`, and `Reservoir` result handlers. The failure output reports `ACTUAL: array([[42., 42., ...` against `DESIRED: array(-3.402823e+38)`. The test pre-fills `D` with `42` and `I` with `99`, so this confirms the output buffers are never written without the fix, rather than written with incorrect values. The passing run is clean under ASan and UBSan. ## Note for issue #3830 The repro in the issue (`d=1`, `nq=20`) no longer reproduces on `main`. When the issue was filed, `distance_compute_blas_threshold` defaulted to `20` and the branch condition was `nx < threshold`, which put `nq=19` on the sequential path (correct sentinels) and `nq=20` on the BLAS path (uninitialized output). The threshold is now dimension-aware (`nx * d < distance_compute_blas_threshold`, default `128000`), so `d=1` requires `nq >= 128000` to select BLAS. The defect was relocated, not fixed. A repro that still holds today is: ``` faiss.IndexFlatL2(128).search(np.random.random((1000, 128)).astype('float32'), 1) # nx * d == 128000 ``` This change removes the defect at its source, for all `nx`, all `d`, both metrics, and all three result handlers. The `k > ntotal` case with `ntotal > 0` was never affected: `begin_multiple` already initializes all `k` slots, so the unfilled tail is correctly padded. Pull Request resolved: #5528 Reviewed By: mnorris11, trang-nm-nguyen Differential Revision: D116650117 Pulled By: alibeklfc fbshipit-source-id: b40bdf7ade7640b2e433765b3877e2cea3546cf2
1 parent e135a1a commit 2832ac3

2 files changed

Lines changed: 33 additions & 2 deletions

File tree

faiss/utils/distances.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,11 @@ struct Run_search_inner_product {
563563
size_t d,
564564
size_t nx,
565565
size_t ny) {
566-
if (res.sel ||
566+
// ny == 0 goes to the sequential path: it guards only on nx, so its
567+
// per-query begin()/end() still runs and each handler writes its own
568+
// neutral distance and -1 label. The BLAS path instead returns early
569+
// on ny == 0, before the handler is initialized.
570+
if (res.sel || ny == 0 ||
567571
nx * d < static_cast<size_t>(distance_compute_blas_threshold)) {
568572
exhaustive_inner_product_seq(x, y, d, nx, ny, res);
569573
} else {
@@ -582,7 +586,8 @@ struct Run_search_L2sqr {
582586
size_t nx,
583587
size_t ny,
584588
const float* y_norm2) {
585-
if (res.sel ||
589+
// See the note on ny == 0 in Run_search_inner_product.
590+
if (res.sel || ny == 0 ||
586591
nx * d < static_cast<size_t>(distance_compute_blas_threshold)) {
587592
exhaustive_L2sqr_seq(x, y, d, nx, ny, res);
588593
} else {

tests/test_index.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,32 @@ def test_empty_query_batch(self):
137137
lims, np.zeros(1, dtype=np.int64)
138138
)
139139

140+
def test_empty_index_with_blas(self):
141+
# The threshold is what makes this a regression test: it puts the
142+
# search in a configuration that would otherwise select the BLAS path,
143+
# which returns early on an empty database without ever initializing
144+
# the result handler. k covers the Top1, heap, and reservoir handlers.
145+
saved_threshold = faiss.cvar.distance_compute_blas_threshold
146+
faiss.cvar.distance_compute_blas_threshold = 1
147+
try:
148+
xq = np.arange(5, dtype="float32").reshape(-1, 1)
149+
for metric_type in (faiss.METRIC_L2, faiss.METRIC_INNER_PRODUCT):
150+
expected_distance = (
151+
np.finfo("float32").max
152+
if metric_type == faiss.METRIC_L2
153+
else np.finfo("float32").min
154+
)
155+
index = faiss.IndexFlat(1, metric_type)
156+
for k in (1, 10, 150):
157+
with self.subTest(metric_type=metric_type, k=k):
158+
D = np.full((len(xq), k), 42, dtype="float32")
159+
I = np.full((len(xq), k), 99, dtype="int64")
160+
index.search(xq, k, D=D, I=I)
161+
np.testing.assert_array_equal(D, expected_distance)
162+
np.testing.assert_array_equal(I, -1)
163+
finally:
164+
faiss.cvar.distance_compute_blas_threshold = saved_threshold
165+
140166

141167
@for_all_simd_levels
142168
class TestDbParallelSearch(unittest.TestCase):

0 commit comments

Comments
 (0)