mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-11 22:50:00 +00:00
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 https://github.com/facebookresearch/faiss/issues/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: https://github.com/facebookresearch/faiss/pull/5528 Reviewed By: mnorris11, trang-nm-nguyen Differential Revision: D116650117 Pulled By: alibeklfc fbshipit-source-id: b40bdf7ade7640b2e433765b3877e2cea3546cf2
This commit is contained in:
committed by
meta-codesync[bot]
parent
e135a1a667
commit
2832ac3f67
@@ -563,7 +563,11 @@ struct Run_search_inner_product {
|
||||
size_t d,
|
||||
size_t nx,
|
||||
size_t ny) {
|
||||
if (res.sel ||
|
||||
// ny == 0 goes to the sequential path: it guards only on nx, so its
|
||||
// per-query begin()/end() still runs and each handler writes its own
|
||||
// neutral distance and -1 label. The BLAS path instead returns early
|
||||
// on ny == 0, before the handler is initialized.
|
||||
if (res.sel || ny == 0 ||
|
||||
nx * d < static_cast<size_t>(distance_compute_blas_threshold)) {
|
||||
exhaustive_inner_product_seq(x, y, d, nx, ny, res);
|
||||
} else {
|
||||
@@ -582,7 +586,8 @@ struct Run_search_L2sqr {
|
||||
size_t nx,
|
||||
size_t ny,
|
||||
const float* y_norm2) {
|
||||
if (res.sel ||
|
||||
// See the note on ny == 0 in Run_search_inner_product.
|
||||
if (res.sel || ny == 0 ||
|
||||
nx * d < static_cast<size_t>(distance_compute_blas_threshold)) {
|
||||
exhaustive_L2sqr_seq(x, y, d, nx, ny, res);
|
||||
} else {
|
||||
|
||||
@@ -137,6 +137,32 @@ class TestIndexFlat(unittest.TestCase):
|
||||
lims, np.zeros(1, dtype=np.int64)
|
||||
)
|
||||
|
||||
def test_empty_index_with_blas(self):
|
||||
# The threshold is what makes this a regression test: it puts the
|
||||
# search in a configuration that would otherwise select the BLAS path,
|
||||
# which returns early on an empty database without ever initializing
|
||||
# the result handler. k covers the Top1, heap, and reservoir handlers.
|
||||
saved_threshold = faiss.cvar.distance_compute_blas_threshold
|
||||
faiss.cvar.distance_compute_blas_threshold = 1
|
||||
try:
|
||||
xq = np.arange(5, dtype="float32").reshape(-1, 1)
|
||||
for metric_type in (faiss.METRIC_L2, faiss.METRIC_INNER_PRODUCT):
|
||||
expected_distance = (
|
||||
np.finfo("float32").max
|
||||
if metric_type == faiss.METRIC_L2
|
||||
else np.finfo("float32").min
|
||||
)
|
||||
index = faiss.IndexFlat(1, metric_type)
|
||||
for k in (1, 10, 150):
|
||||
with self.subTest(metric_type=metric_type, k=k):
|
||||
D = np.full((len(xq), k), 42, dtype="float32")
|
||||
I = np.full((len(xq), k), 99, dtype="int64")
|
||||
index.search(xq, k, D=D, I=I)
|
||||
np.testing.assert_array_equal(D, expected_distance)
|
||||
np.testing.assert_array_equal(I, -1)
|
||||
finally:
|
||||
faiss.cvar.distance_compute_blas_threshold = saved_threshold
|
||||
|
||||
|
||||
@for_all_simd_levels
|
||||
class TestDbParallelSearch(unittest.TestCase):
|
||||
|
||||
Reference in New Issue
Block a user