mirror of
https://github.com/facebookresearch/faiss.git
synced 2026-10-11 22:50:00 +00:00
main
2354
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 YingzuoLiu
|
3778f5892d |
Fix IDSelector handling in unbounded HNSW search (#5508)
Summary: - apply `SearchParametersHNSW.sel` to the unbounded HNSW search path - keep filtered nodes available for graph traversal while excluding them from the result set - add regression coverage for L2 and inner-product search, restrictive `efSearch`, and an empty selector ## Root cause The bounded HNSW path applies the selector through its result handler. The unbounded path did not receive the selector and used the same heap for traversal pruning and final results, so IDs rejected by `SearchParameters.sel` could be returned. ## Implementation When a selector is present, the unbounded path maintains a separate result heap containing only accepted IDs. The existing candidate heap remains unchanged and can still contain rejected nodes, preserving traversal through them. When no selector is present, the existing unfiltered result behavior is preserved. ## Validation - issue reproducer: unbounded search now returns ID 1 at distance 100.0 instead of rejected ID 0 - `tests/test_search_params.py`: 52 passed - HNSW C++ tests: 14 passed - full CPU C++ test suite: 278 passed, 5 skipped - clang-format and `git diff --check`: passed Fixes https://github.com/facebookresearch/faiss/issues/5504 Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5508 Reviewed By: alibeklfc Differential Revision: D120364656 Pulled By: mnorris11 fbshipit-source-id: 074234b80cf7b2e06a85d57489a1c214dfd3195b |
||
|
Alibek Zhakubayev
|
1dbb3d25b0 |
Update backward compatibility test to v1.15.1 (#5646)
Summary: - Update backward compatibility test to use faiss-cpu=1.15.1 Note: the test was previously pinned to faiss-cpu=1.14.3, so this skips over 1.15.0. **Do not land until faiss-cpu=1.15.1 is available on the pytorch conda channel** — the conda package is published by the tag-triggered release build. Landing earlier will break this workflow on main. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5646 Test Plan: - [ ] CI passes - [ ] Backward compatibility test uses correct version Reviewed By: mnorris11 Differential Revision: D120265026 Pulled By: alibeklfc fbshipit-source-id: 9459958af352716d5aef25e45043444105f22ff1 |
||
|
Alibek Zhakubayev
|
75c755c5a1 |
Add missing CHANGELOG entry for #5632 in v1.15.1 (#5651)
Summary: - https://github.com/facebookresearch/faiss/issues/5632 (Add explicit IVFPQ scanner distance modes) landed between the changelog generation and the version bump in https://github.com/facebookresearch/faiss/issues/5645, so it ships in 1.15.1 but was missing from `CHANGELOG.md`. - Adds it under `Added` in the 1.15.1 section. Changelog-only; no code changes. **Time-sensitive:** the v1.15.1 tag will be cut from the latest `main`. Landing this before tagging gets the entry into the released changelog. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5651 Test Plan: - [ ] CI passes (docs-only change) Reviewed By: mnorris11 Differential Revision: D120360726 Pulled By: alibeklfc fbshipit-source-id: 2854a7c44958ec4528ec87804382c312ea195ad2v1.15.1 |
||
|
Alibek Zhakubayev
|
d63faeaf5a |
Increment to next release, v1.15.1 (#5645)
Summary: - Increment version to 1.15.1 - Update CHANGELOG.md with commits since v1.15.0 (57 commits) - Bump version in CMakeLists.txt, setup.py, Index.h, INSTALL.md Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5645 Test Plan: - [ ] CI passes - [ ] CHANGELOG entries are correctly categorized Reviewed By: mnorris11 Differential Revision: D120264788 Pulled By: alibeklfc fbshipit-source-id: d95d68c9eb0c7c5b06aaeab7d8d0015b99d23145 |
||
|
Jaeyoon Jeong
|
3b3711c112 |
Add explicit IVFPQ scanner distance modes (#5632)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5632 Add an explicit IVFPQ scanner mode that preserves precomputed-table behavior by default and can decode PQ codes to compute residual L2 distances on demand. Keep the existing scanner factory overload compatible. Reviewed By: mdouze Differential Revision: D119608525 fbshipit-source-id: b60b2c9771bdfd95d9bee2e81b72c9c3e7abb82c |
||
|
Alibek Zhakubayev
|
3bb48643ab |
Pin faiss-gpu-cuvs RAPIDS deps to the 26.06 build series (#5643)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5643 `faiss-gpu-cuvs` 1.15.0 fails at import on a clean install: ``` ImportError: libfaiss.so: undefined symbol: _ZTIN3rmm9bad_allocE ``` ## Root cause, measured RAPIDS 26.08 wrapped RMM in a per-release ABI inline namespace. `nm -D` on the published `librmm-cu13` wheels: | Version | `typeinfo for rmm::bad_alloc` | Exported instead | | --- | --- | --- | | 26.2.0 | present | | | 26.4.0 | present | | | 26.6.0 | present | | | 26.8.0 | **absent** | `typeinfo for rmm::_RMM_26_8::bad_alloc` | `pyproject-gpu-cuvs.toml` builds against `==26.06.*` in `before-all` but declares `>=26.06,<27` at runtime and in `test-requires`. A fresh install therefore pairs a 26.06-built `libfaiss.so` with a 26.8 `librmm.so` that renamed every `rmm::` symbol. Nothing vendored covers the gap: `repair-wheel-command` hands every RAPIDS `lib*.so*` under `/opt/cuvs` to `auditwheel --exclude`. The 1.15.0 wheel was uploaded 2026-08-03, two days before RAPIDS 26.08 reached PyPI. It published green and began failing for every new installer afterwards, which matches `build-pip-gpu / Build & test faiss-gpu-cuvs` going red on 2026-08-05 after 68 consecutive passes. An exact-series pin rather than a narrower range is required: the inline namespace encodes the release, so **any** range spanning more than one RAPIDS minor series is broken by construction. ## This diff - Pin `libcuvs-cu13`, `libraft-cu13`, and `librmm-cu13` to `==26.06.*` in both `dependencies` and `test-requires`. - Declare `libraft-cu13`. `faiss/python/__init__.py` imports `libraft` and calls `libraft.load_library()`, but only `before-all` installed it, so at runtime its version floated on `libcuvs-cu13` transitive metadata. - Correct the cuSOLVER soname in the `auditwheel` exclude list from `libcusolver.so.11` to `libcusolver.so.12`, which is what CUDA 13.2 ships. - Update the `_preload_gpu_libs` fix-it message, which told users to install the range this diff removes. - Add `TestCuvsRapidsPinning` to `tests/test_wheel_smoke_gpu.py`. ## Regression guard `tests/test_wheel_smoke_gpu.py` is already the `test-command` for the cuVS wheel, so the new class runs inside `build-pip-gpu.yml` with no workflow change and no new test file. It asserts three properties of the built wheel's own `Requires-Dist`: 1. every RAPIDS distribution `_preload_gpu_libs` dlopens is declared 2. each is pinned to `==26.06.*` rather than an open range 3. the installed versions sit in that series `_RAPIDS_SERIES` duplicates the series from `pyproject-gpu-cuvs.toml` because the wheel does not ship that file. The duplication is the tripwire: a RAPIDS bump has to touch both places deliberately. ## Prior attempt D117099095 made the version-pin change on 2026-08-23 and was abandoned on 2026-09-04 without landing. It never failed review. `faiss/public_tld/` is ShipIt-mirrored with `requireExportConfig`, so `github-export-checks` held it Unpublished pending a `facebookresearch/faiss` PR that a bot cannot open, and three pings over 12 days went unanswered. This diff needs that same export. ## Notes The published 1.15.0 metadata declares only `libcuvs-cu13`, not `librmm-cu13`. That is not a fbsource/GitHub divergence: `librmm-cu13` was added to `dependencies` on 2026-08-14, after the 08-03 release, and has never shipped. `rapids-logger` is installed by `before-all` but stays undeclared here. `librmm-cu13` owns that link-time dependency and carries it in its own metadata; an unversioned duplicate would not make the resolve safer. Reviewed By: mnorris11 Differential Revision: D120162004 fbshipit-source-id: eb6dc316a4fba2a4228eab62d7b86ac8ea94a1b7 |
||
|
Trang Nguyen
|
7c3b55c01b |
Make Panorama bounds checks overflow-safe (#5616)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5616 Follow up on D118495504 by closing two arithmetic gaps in the Panorama bounds validation. Avoid signed overflow and reject negative counts in `IndexFlatPanorama::reconstruct_n`, and compute the serialized `IndexFlatPanorama` batch count without overflowing `ntotal + batch_size - 1`. Add regression coverage for negative reconstruction counts and a maximal serialized batch size that previously admitted empty storage for a non-empty index. Reviewed By: mnorris11 Differential Revision: D119272683 fbshipit-source-id: 7ea841bd4bd90d0358b78797441fa3cc815db46a |
||
|
Trang Nguyen
|
fb2d57a312 |
Fix leak and unbounded lists[] in OnDisk invlist deserialization (#5560)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5560 `OnDiskInvertedListsIOHook::read` allocates its `OnDiskInvertedLists` with a bare `new` and then keeps reading. Any throw after that point leaks the whole object -- including `od->lists`, which `READVECTOR` will happily size up to `get_deserialization_vector_byte_limit()`. Throwing after that point is the common case for malformed input: the `slots` and `filename` reads can fail, and `do_mmap()` on an attacker-controlled filename fails essentially always. This is what the `faiss_read_index_binary_fuzzer` health check has been tripping over. The one CI stack that survived tpx truncation names `OnDiskInvertedLists.cpp:722` (`vector<OnDiskOneList>::resize`) as the largest live allocation, reached via `IBFf` -> `read_index` -> `IxRP` -> IVF -> `read_InvertedLists`. Because the harness runs with `detect_leaks: 0`, the leaked blocks accumulate across a 120 s session; peak RSS across the failing runs climbed 978 -> 1142 -> 1249 -> 1815 -> 4356 MB against `-rss_limit_mb=4096`. That is why the check reads as flaky (23.6%) rather than as a hard red, and why the saved crash artifacts do not reproduce when replayed one at a time. Switch both `read` and `read_ArrayInvertedLists` to hold the object in a `std::unique_ptr` and `release()` on success. The public signature still returns `InvertedLists*`, so this is a local scope guard only -- the same shape `read_InvertedLists_up` in `index_read.cpp` already uses. Also reject `lists.size() != nlist` at read time. Every `OnDiskInvertedLists` accessor (`list_size`, `get_codes`, `get_ids`, ...) indexes `lists[list_no]` with `list_no` bounded only against `nlist`, so a file whose `lists` vector is shorter than its `nlist` is a straight out-of-bounds read. Every construction path in the class keeps `lists.size() == nlist`, so this rejects only malformed input. Reviewed By: mnorris11 Differential Revision: D118487565 fbshipit-source-id: 6f01c937b24a2a98620e4ca2bde141674af1b19d |
||
|
Michael Norris
|
a74867cf56 |
faiss HNSW: delete the flag and the lock-based graph build (#5611)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5611 D112025877 added the `faiss::hnsw_deterministic_build` flag. The flag chose between the lock-based graph build and the deterministic graph build. This diff deletes the flag. The deterministic build is now the only build. Reviewed By: trang-nm-nguyen Differential Revision: D118342471 fbshipit-source-id: d3c5e5238aca2bcda463eb27959ec22827222c15 |
||
|
Ge Gao
|
4fdbc9dc18 |
Support ZeroCopyIOReader for IndexRaBitQ deserialization (#5629)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5629 `IndexRaBitQ` keeps its database in `codes`, a `MaybeOwnedVector<uint8_t>` inherited from `IndexFlatCodes`, but both read branches (`Ixrq` for single-bit and `Ixrr` for multi-bit) used the raw `READVECTOR` macro. So `codes` was always an owning heap vector even when the caller passed a view-capable reader, while every other flat-codes index already goes through `read_vector`. This switches both branches to `read_vector`, which dispatches on the reader type in `read_vector_base` and makes `codes` a view of the caller's buffer. Any other reader falls back to `READVECTOR`, so existing callers are unaffected. A zero-copy RaBitQ index now works like this: ``` #include <faiss/impl/zerocopy_io.h> faiss::ZeroCopyIOReader reader(buffer.data(), buffer.size()); std::unique_ptr<faiss::Index> index_zc(faiss::read_index(&reader)); ``` The buffer must outlive the index, as with any `faiss::ZeroCopyIOReader`. Reviewed By: mnorris11 Differential Revision: D119297402 fbshipit-source-id: 95c5ba050cb3a0636a0899efbe886616a7a8a2fd |
||
|
Michael Norris
|
b5a14632a6 |
Run the eight SIMD kernel tests that no build executed (#5620)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5620 **TL;DR:** Eight SIMD kernel tests never ran, and two byte-domain kernels trust their caller. This change makes three fixes: 1. **Six tests never compiled.** `test_rabitq_simd.cpp` hides them behind `COMPILE_SIMD_*` macros that the faiss target does not export. 2. **Two tests skipped themselves in a static build.** `TestSQByteDirectAcrossSIMDLevels` needs `SIMDLevel::NONE` as a reference, and a static build does not report that level. 3. **The byte kernels never test their dimension requirement.** `DistanceComputerByte` and `DistanceComputerByteSigned` read 16 bytes per loop step. ## 1. Six tests never compiled `test_rabitq_simd.cpp` wraps two blocks in `#ifdef COMPILE_SIMD_AVX2` and `#ifdef COMPILE_SIMD_AVX512`. The faiss target sets those macros as `preprocessor_flags`, and it does not export them. A test translation unit never sees them. So the preprocessor deleted both blocks. The file holds nine tests and ran three. The AVX2 block loses its guard, because every x86 faiss build holds the AVX2 kernels. Its three tests also lose an `#ifdef FAISS_ENABLE_DD` around a runtime skip, which the same problem disabled. `is_simd_level_available()` reports the right answer in a static build too. The AVX512 block moves to a dynamic dispatch target. A static x86 build compiles the AVX2 kernels only, so the test fails to link there rather than skips. `test_rabitq_simd_vpopcnt.cpp` becomes `test_rabitq_simd_dd.cpp`, because it now holds every RaBitQ test that a static build cannot link. A new `test_rabitq_simd_util.h` holds the scalar reference that both files call. ## 2. Two tests skipped themselves in a static build `TestSQByteDirectAcrossSIMDLevels` measured the index a second time at `SIMDLevel::NONE`, then compared the two runs. A static build reports only the level it compiles, and omits `NONE`, so both tests skipped. The tests now compute the distances from the definition. The distances are integers, so the reference is exact. The tests check absolute values now, instead of the agreement of two faiss paths. ## 3. The byte kernels trust the caller `DistanceComputerByte` and `DistanceComputerByteSigned` read 16 bytes per loop step, so they require `d % 16 == 0`. `sq-dispatch.h` enforces that, and no other caller builds them. A caller that ignores the requirement reads up to 15 bytes past each code and returns a wrong distance. Each constructor now tests the requirement and throws. The AVX2 and the ARM_NEON specializations both get the test. Reviewed By: alibeklfc Differential Revision: D119545379 fbshipit-source-id: ddfdcc30d3662f469b2ec3dbb9d5b9f4dafa21aa |
||
|
Michael Norris
|
6bbb068ad8 |
Measure 20-byte Hamming codes eight at a time (#5587)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5587 **TL;DR:** Adds a batched 20-byte Hamming kernel that measures eight codes per call, the only thing that helps the 160-bit width, worth 1.21x end to end. A 20-byte code has no vector kernel at any x86 SIMD level. `HammingComputer20` measures one distance with three scalar popcounts at every level, including `AVX512_VPOPCNT`, so a scan of 160-bit codes runs the same code everywhere. Vectorizing a single 20-byte distance does not help. A masked 256-bit load plus `vpopcntq` plus a horizontal reduction measures the same as the scalar body, because the reduction costs more than the three popcounts it replaces. Measuring several codes per call does help, because it amortizes that reduction. This adds `hamming_batch()` to `HammingComputer20` at `AVX512_VPOPCNT`, which measures eight codes at once: - Eight codes span 160 bytes. `_mm512_popcnt_epi8` gives a count per byte. - `_mm512_sad_epu8` sums each aligned 8-byte group. - A 20-byte code covers two whole groups plus half of a group it shares with its neighbour, so each code needs one scalar popcount for its half. `IVFBinaryScannerL2::scan_codes` and `IVFBinaryScannerL2::scan_codes_range` call `hamming_batch()` when the computer declares a `batch_size`, then measure the remaining codes one at a time. A computer without a `batch_size` keeps the existing loop, so no other code size changes. The scanner holds the tiled query, and the constructor rejects a code size that does not match the computer. A caller therefore cannot reach the batch kernel with a stride the kernel does not read. The byte-wise popcount needs `AVX512_BITALG`, which is a separate CPUID bit from `AVX512_VPOPCNTDQ`. This adds the flag to the `AVX512_VPOPCNT` level and requires both bits to select it. Every CPU that has VPOPCNTDQ also has BITALG, so no CPU loses the level. An alternative byte-wise popcount built from `vpshufb`, which needs neither bit and would run at the plain `AVX512` level, measures only 1.08x against 1.54x, so it is not worth the wider reach. Reviewed By: alibeklfc Differential Revision: D118971285 fbshipit-source-id: 5fd4637c972e5c55e2695b32b9b15fba8181a242 |
||
|
Michael Norris
|
3c7042f17a |
Let the binary Hamming paths reach the VPOPCNT kernels
Summary: **TL;DR:** Points `IndexBinaryIVF` at the VPOPCNT level and instantiates its scanner in the translation unit compiled with `-mavx512vpopcntdq`, worth 1.58x at 256-bit and 1.41x at 512-bit on Genoa. Two things kept the binary Hamming paths on scalar popcount, whatever the host. `IndexBinaryIVF` builds its scanner and runs its searches through `with_simd_level`, which uses the BASE level mask. That mask holds no `AVX512_VPOPCNT` bit, so on a host that reports that level the dispatch falls through to `AVX512`, where the wide computers use scalar popcount. Two build variants, `faiss_omp_mock` and `faiss_no_multithreading`, hardcoded dynamic dispatch off. They compiled the AVX2 kernels only, so a consumer of either variant could not use runtime dispatch at all, whatever the build setting said. This change: - Moves the three dispatch sites in `IndexBinaryIVF.cpp` to `with_simd_level_with_vpopcnt`. Below that level the dispatch falls through to `AVX512`, as before. - Instantiates `IndexBinaryIVF_impl.h` in `hamming_avx512_vpopcnt.cpp`. That is the one translation unit compiled with `-mavx512vpopcntdq`, which the VPOPCNT computers need. Without it the new dispatch has no symbol to call. The include must follow the computer specializations, so it is placed after them. - Lets both build variants honour the dynamic dispatch setting. `HammingComputer16` and `HammingComputer20` still inherit the scalar body at this level, so a 16-byte or 20-byte code gains nothing here. `IVFBinaryScannerL2::scan_codes` also reads the heap top into a local `bound` instead of loading `simi[0]` on every code. Only `scan_codes` writes `simi`, so it refreshes `bound` after each `heap_replace_top` and the two forms return the same results. The test plan measures it. Reviewed By: alibeklfc Differential Revision: D118874433 fbshipit-source-id: dc74294ecaf178919fdd713abc60c44bbaa80347 |
||
|
lyang24
|
e6bd1d252c |
Add AVX512 VPOPCNT dynamic dispatch level (#5531)
Summary: ## What Split the existing VPOPCNTDQ RaBitQ and Hamming kernels from the full Sapphire Rapids SIMD level. DD builds now expose an `AVX512_VPOPCNT` capability for CPUs such as Ice Lake and Zen 4. The fallback chain is: `AVX512_SPR -> AVX512_VPOPCNT -> AVX512 -> AVX2 -> NONE` The ordinary static AVX512 target remains unchanged and contains no VPOPCNT instructions. ## Why The VPOPCNT kernels only require baseline AVX-512 plus `AVX512_VPOPCNTDQ`. Tying them to the SPR level unnecessarily excluded CPUs that support VPOPCNTDQ but not AVX512-FP16, notably Zen 4. ## Changes made while importing Two changes were needed on top of the pull request. **Feature detection read the wrong CPUID leaf.** `AVX512_VPOPCNTDQ` and `AVX512_VNNI` were read from leaf 1 ECX. Both live in leaf 7 subleaf 0 ECX, at bits 14 and 11. Leaf 1 ECX holds unrelated bits at those positions, so the new level was never selected on any CPU. Measured on an AMD Genoa host before the fix: the level reported as unavailable and detection returned `AVX512`. After the fix the same host reports `AVX512_VPOPCNT`. **The build definitions outside CMake were not updated.** The pull request renames three files. The other build description still named the old paths and had no entry for the new level, so the library did not build. This adds the level constant, its compiler flags, its `COMPILE_SIMD_AVX512_VPOPCNT` define, and the renamed paths. **The new level was not registered in the compiled-level mask.** `compiled_simd_levels()` reports which levels a binary holds, and `FAISS_SIMD_LEVEL` walks down from the requested level until it finds one that is present. The new level had no entry, so the override always stepped past it to `AVX512` and printed that the level was not compiled in. This was reported by `test_dispatch_with_env_var` on the open-source dynamic-dispatch build. **The test helper inferred available levels from `/proc/cpuinfo`.** A CPU feature does not imply that a level is compiled into the build, so the helper listed a level the override then refused. It now asks `SIMDConfig::is_simd_level_available()`, which reports compiled and supported. That is a subset of compiled, so every level it returns is one the override accepts. The mask and helper names also follow the naming used in the parent diff, so `AVAILABLE_SIMD_LEVELS_BASE_WITH_VPOPCNT` and `with_simd_level_with_vpopcnt`. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5531 Test Plan: ``` buck2 test fbcode//mode/opt -c faiss.dynamic_dispatch=true \ fbcode//faiss/tests:test_simd_levels fbcode//faiss/tests:test_rabitq_simd ``` Result: all pass. The build failures the imported version reported are cleared. All three configurations build: ``` buck2 build fbcode//mode/dev fbcode//faiss:faiss fbcode//faiss:faiss_no_multithreading fbcode//faiss:faiss_omp_mock buck2 build fbcode//mode/opt -c faiss.dynamic_dispatch=true <same three targets> buck2 build fbcode//mode/dev -c fbcode.arch=aarch64 <same three targets> ``` Detection on an AMD Genoa host, which has AVX512_VPOPCNTDQ and AVX512_BF16 but not AVX512_FP16: | | before the CPUID fix | after | | --- | --- | --- | | detected level | `AVX512` | `AVX512_VPOPCNT` | | `AVX512_VPOPCNT` available | 0 | 1 | | `AVX512_SPR` available | 0 | 0 | Reviewed By: alibeklfc Differential Revision: D118969754 Pulled By: mnorris11 fbshipit-source-id: d1ca52b614829a3d4b322c9fecf0c004f2174dab |
||
|
Michael Norris
|
9f993087b0 |
Name the SIMD level masks after what they hold (#5600)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5600 **TL;DR:** Renames the dispatch masks from `A0`/`A1`/`A2` to names that say which levels they hold; no behaviour change. The dispatch masks were named `A0`, `A1` and `A2`. The names say nothing about which levels a mask holds, so a caller had to read `simd_dispatch.h` to pick one. That is how several call sites ended up on a mask with no `ARM_SVE` bit while a dedicated SVE kernel existed. This renames every mask and helper after its contents. It changes no behaviour. | Old name | New name | Holds | | --- | --- | --- | | `AVAILABLE_SIMD_LEVELS_A0` | `..._BASE` | NONE, AVX2, AVX512, ARM_NEON, RISCV_RVV | | `AVAILABLE_SIMD_LEVELS_A0_SPR` | `..._BASE_WITH_SPR` | BASE + AVX512_SPR | | `AVAILABLE_SIMD_LEVELS_A1` | `..._BASE_WITH_SVE` | BASE + ARM_SVE | The helpers follow: `with_simd_level_a0_spr` becomes `with_simd_level_with_spr`, and `with_simd_level_a1` becomes `with_simd_level_with_sve`. `BASE` is the right name for the default mask because `ARM_NEON` belongs to it. NEON is mandatory on aarch64, the way AVX2 is the x86 baseline, while `ARM_SVE` is optional. So the ARM fallback chain is SVE to NEON to NONE. Reviewed By: alibeklfc Differential Revision: D119025991 fbshipit-source-id: 2b6681de2d83df6468569a365d05341088742cf8 |
||
|
Michael Norris
|
fc13c81fb8 |
Stop aarch64 falling back to SIMDLevel::NONE in faiss dispatch (#5572)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5572 **TL;DR:** Stops aarch64 running scalar code where a NEON or SVE kernel exists, adds the two missing NEON byte-domain kernels, and fixes `FAISS_SIMD_LEVEL` falling all the way to `NONE` on an uncompiled level. On aarch64 several faiss dispatch paths ran scalar code, or ran an `ARM_NEON` kernel where an `ARM_SVE` kernel already existed. This change enforces the order SVE -> NEON -> NONE on an ARM host. It also adds the one kernel pair that was missing. T287037898 reported the scalar-quantizer half. On aarch64 `IndexScalarQuantizer` with `QT_8bit_direct` or `QT_8bit_direct_signed` saved memory but lost throughput. `Refine(SQ8)` has the same problem, because `IndexRefine::search` calls `refine_index->get_distance_computer()`. ## What each call site gets | Call site | Before | After | | --- | --- | --- | | `QT_8bit_direct` on aarch64 | float-domain `DCTemplate` | NEON `DistanceComputerByte` | | `QT_8bit_direct_signed` on aarch64 | float-domain `DCTemplate` | NEON `DistanceComputerByteSigned` (new) | | `IndexFlat` distance computers, SVE host | `ARM_NEON` | `ARM_SVE` | | `AdditiveQuantizer::compute_centroid_norms`, SVE host | `ARM_NEON` | `ARM_SVE` | | `SuperKMeans` `block_l2`, SVE host | `ARM_NEON` | `ARM_SVE` | | `pq_code_distance` wrappers, SVE host | `ARM_NEON` | `ARM_SVE` | | `with_VectorDistance`, SVE host | `ARM_NEON` | `ARM_SVE` | | `FAISS_SIMD_LEVEL=ARM_SVE`, build without SVE | `NONE` | nearest compiled level | ## Three causes, fixed separately **1. Two missing kernels.** `sq-neon.cpp` held a scalar `DistanceComputerByte<Sim, ARM_NEON>` that nothing could reach, and no `DistanceComputerByteSigned<Sim, ARM_NEON>` at all. Both are now real NEON kernels. L2 and the unsigned inner product use `vabdq_u8`, `vmull_u8` and `vpadalq_u16`. The bias-encoded inner product uses `veorq_u8`, `vmull_s8` and `vpadalq_s16`. Both agree bit for bit with the AVX2 specializations, which the tests require. Two invariants deserve a note: - The L2 path keeps an unsigned accumulator. A `vmull_u8` square reaches 255^2 = 65025, which an int16 lane would read as negative. - For x in 0 to 255, `x ^ 0x80` read as `int8` is exactly `x - 128`. That is how the kernel removes the +128 bias before `vmull_s8`. **2. Dispatch chains that list only x86 levels.** The `if constexpr` chains in `sq-dispatch.h` enumerated x86 levels only, so an ARM host fell through to the float path. This adds `ARM_NEON` to four chains: two in `select_distance_computer_body`, and two in `sq_select_InvertedListScanner`. The chain in `is_dimension_compatible` already included ARM. This does not add `ARM_SVE`. The scalar-quantizer entry points dispatch with a mask that holds no `ARM_SVE` bit, so an SVE host already falls through to the `ARM_NEON` case and now gets these kernels. Adding `ARM_SVE` would instantiate the empty primary template in `distance_computers.h`. This corrects that template's stale comment. **3. Level masks that hide existing SVE kernels.** The default mask holds no `ARM_SVE` bit, so the dispatch fell through `case ARM_SVE` to `ARM_NEON`. This uses `with_simd_level_a1` at every site where a real SVE kernel exists and links: `IndexFlat.cpp` at four sites, `AdditiveQuantizer::compute_centroid_norms`, `block_l2` in `SuperKMeans.cpp`, all three wrappers in `pq_code_distance-generic.cpp`, and `with_VectorDistance` in `distances_dispatch.h`. The `IndexFlat` sites matter most. `faiss::fvec_L2sqr()` already used the SVE mask, so on an SVE host the free function ran SVE while `IndexFlatL2`'s distance computer ran NEON. Also: `FAISS_SIMD_LEVEL=ARM_SVE` on a build without SVE compiled in used to skip every level and run at `NONE`, because `with_selected_simd_levels` has no case label for an uncompiled level. It now walks down to the nearest compiled level. The override is still honoured when the CPU lacks the level, since forcing a level is the point of it. Only uncompiled levels are corrected. The unused `AVAILABLE_SIMD_LEVELS_A2` is deleted. It is the NEON-to-NONE trap in constant form, with no users. This change keeps the existing mask names. A follow-up renames them to say what they hold. ## Out of scope In rough order of remaining value: - `IndexPQ.cpp` and `IndexIVFPQ.cpp` still pin PQ to `ARM_NEON` on an SVE host. A mask change is not enough. `pq_code_distance-sve.cpp` includes only `pq_scan_impl.h`, and `with_HammingComputer<ARM_SVE>` has no complete type. - `distances_aarch64.cpp` forwards five `ARM_NEON` specializations to `<SIMDLevel::NONE>`, so a non-SVE aarch64 host runs scalar code in the IVFFlat scan. - `rabitq_neon.cpp` forwards all six `ARM_NEON` specializations to `<SIMDLevel::NONE>`. There is no SVE variant. - There is no `block_l2<ARM_NEON>` and no `exhaustive_L2sqr_blas_cmax<ARM_NEON>`. - Part two of T287037898, which is SDOT and SMMLA. `FEAT_DotProd` and `FEAT_I8MM` are not expressible as a `SIMDLevel`. The right shape is a runtime `getauxval(AT_HWCAP)` check inside the ARM translation unit, which follows the `SIMDConfig::avx512_split` precedent. Note the reporter's caveat: SMMLA shows no improvement until the scan loop is tiled, so the tiling must land in the same change. Reviewed By: alibeklfc Differential Revision: D118682598 fbshipit-source-id: 1ab588d9fa130e8246bf996025fc11d8954323be |
||
|
Michael Norris
|
00928e3f76 |
Fix ROCm CI: build in a dedicated conda env instead of the runner's broken base (#5545)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5545 The `linux-x86_64-GPU-w-ROCm-cmake` job fails installing AMD's ROCm torch wheels: they exist for one CPython ABI only, and the MI325 runner's interpreter is 3.14, not the 3.12 the action asks for. Nothing in `build_cmake` actually constrained the interpreter. `setup-miniconda` requests `python-version: '3.12'` and `Configure build environment` asks for `python=3.12` again, but the runner's conda base has `conda-meta` listing both `python 3.12.9` and `cpython 3.14.6`, so conda reports "already installed" while `bin/python` is 3.14. Forcing it does downgrade the interpreter, but strands conda's own compiled extensions (`pydantic_core`), and conda builds its argument parser through its plugins -- so every subsequent conda call in the job dies. Working around that with `CONDA_NO_PLUGINS` in turn drops the libmamba solver, which conda then rejects because `solver: libmamba` stays configured. Meanwhile the base env is inconsistent across three Python ABIs (conda lists ~200 conflicting packages), so there is no in-place repair to converge on. So stop repairing base. For ROCm only, `conda create` a fresh `$CONDA/envs/faiss` prefix on 3.12 and point `$CONDA` at it, leaving base untouched so conda keeps running on its own interpreter. `$CONDA` is the single lever the later steps already read (`-DPYTHON_EXECUTABLE`, `setup.py install`, the import smoke tests), so redirecting it is enough. Making that lever actually hold: - Pass `-p "$CONDA"` on every `conda install`, and `conda activate "$CONDA"`. A bare `conda install` targets the active env, which is base regardless of `$CONDA`. No-op for the non-ROCm jobs, where `$CONDA` is base. - Write base's `bin` to `$GITHUB_PATH` *before* the env's. The runner prepends entries as it reads them, so the last line written ends up first on PATH: this leaves the env's interpreter ahead of base's while keeping base reachable for `conda` itself. - Drive the ROCm torch step through `"$CONDA/bin/python" -m pip` rather than bare `python`/`pip3`, so it does not depend on PATH ordering at all. - `rm -rf` the prefix before `conda create`: it survives between runs on this self-hosted runner and `conda create` refuses an existing one. A fresh prefix each run is the point. - Derive the `libpthread_nonshared.a` symlink from `$CONDA` instead of a hardcoded `$HOME/miniconda3`, since `sysroot_linux-64` now installs into the build's prefix, and assert the target exists -- `ln -s` does not check, so a wrong path would surface much later as a link failure. - `conda list -p "$CONDA"` for the diagnostics, so the logs describe the env under test rather than base. Torch wheel installation: - Resolve through `--find-links` as a flat index instead of hardcoding a `cp312-cp312-linux_x86_64` filename; the hardcoded tag is what made an interpreter mismatch surface as "not a supported wheel on this platform". - Keep a retag fallback for pip builds that reject the unqualified `linux_x86_64` tag, but gate it on the resolver's own error text so a transient failure (network, disk) fails loudly instead of being retagged over. - `--ignore-installed`, because this runner is self-hosted and a wrong-ABI torch from an earlier run otherwise satisfies the requirement and skips tag matching. - Use AMD's wheels built against the system ROCm rather than the PyTorch Foundation `rocm7.2` wheels, which bundle an older ROCr lacking `hsa_amd_memory_get_preferred_copy_engine` and collide with the system `libamdhip64`. `ROCM_VERSION` is exported to `$GITHUB_ENV` so the apt release and the wheel release cannot drift apart. - Assert on interpreter version, torch's install location, and `torch.version.hip` rather than printing them. Each of these failures previously surfaced several steps later as something unrelated-looking. Note for infra: the base env on this runner is corrupt independently of this change (mixed 3.12/3.13/3.14 packages, and truncated `lib/python3.1/site-packages` paths from a `sys.version[:3]` bug upstream). Wiping it would let several of the workarounds above be dropped. Reviewed By: alibeklfc Differential Revision: D117896658 fbshipit-source-id: 26f98c53e5daf747dd81e0fd35232b4160f62377 |
||
|
Trang Nguyen
|
5702cafcb7 |
Remove dead TurboQuant projection state (#5618)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5618 Remove the unused projection buffers and eager `init_projection` work from `ScalarQuantizer::TurboQuantRefine`. Quantizers construct the required projection from `trained` when used, so rebuilding it during training and deserialization only added an unbounded quadratic allocation and cubic QR operation. Keep `seed` and `qjl_type` as the serialized configuration, enforce that the helper remains a small config-only struct, and cover both projection modes in deserialization tests. ___ Differential Revision: D119243693 fbshipit-source-id: 53f982b00dd849cbe88983ffd27809c2fa738a4c |
||
|
Trang Nguyen
|
545550f6a2 |
Use unittest assertions and f-strings in test_index_composite (#5571)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5571 `test_index_composite.py` used bare `assert` statements throughout. Inside a `unittest.TestCase` these are worse than the `self.assert*` helpers in two ways: a failure prints only `AssertionError` with no operand values, so you have to re-run under a debugger to see what the actual number was, and the whole assertion is compiled out under `python -O`, silently turning the test into a no-op. This converts every bare `assert` in the file to the matching `self.assert*` call (`assertEqual`, `assertTrue`, `assertFalse`, `assertNotEqual`, `assertGreater`) and replaces the two remaining printf-style format strings with f-strings. Mechanical change only — no assertion was strengthened, weakened, or reordered, and no test logic was touched. The `raise AssertionError()` idioms guarding `try`/`except RuntimeError` blocks are left alone; converting those to `assertRaises` would change control flow and belongs in its own diff. Split out of D118606761 so that the `IndexIVFFlatDedup::remove_ids` bug fix stays reviewable on its own. Note this file is exported to the open source facebookresearch/faiss repo, so this diff needs an OSS export alongside it. Reviewed By: mnorris11 Differential Revision: D118721255 fbshipit-source-id: b6933a6119955f2dbd7590f4e39632d700a537c4 |
||
|
Trang Nguyen
|
1e5780fbbf |
Fix OOB read in Panorama::reconstruct via IndexPreTransform (T287094917) (#5562)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5562 Lionhead harness `faiss_rmdt_post_read_reconstruct_fuzzer` found a heap out-of-bounds read at `Panorama.cpp:171`, reported as `READ of size 5122903332` in `__asan_memcpy`. Decoding the 232-byte crash input, it is `IxPT -> IxPT -> IxPT -> IxFp` where every `IndexPreTransform` carries an **empty** transform chain and the innermost `IndexFlatIPPanorama` has `d = 0x4C565349 = 1280725833` (so `code_size = d * 4 = 5122903332`, exactly the ASAN read size), `ntotal = 0`, and an empty `codes` vector. Three independent defects combine: 1. `IndexFlatPanorama::reconstruct` never bounds-checks `key`. It overrides `IndexFlat::reconstruct`, which does have `FAISS_THROW_IF_NOT(key < ntotal)`, and drops the guard. Overriding `reconstruct_n` likewise removes `IndexFlatCodes::reconstruct_n`'s `i0 + ni <= ntotal` check. With `ntotal == 0` and empty `codes`, `reconstruct(0, ...)` memcpy's `code_size` bytes out of a zero-length buffer. `Panorama::reconstruct` takes an unsized `const uint8_t*`, so it cannot self-validate -- the caller has to. 2. The `IxFP`/`IxFp` read branch does not range-check `d`. It is the one index type that does not go through `read_index_header`, so it misses that function's `FAISS_CHECK_RANGE(d, 0, (1 << 20) + 1)` and `ntotal >= 0` checks. The `codes.size()` equality check below it does not catch this, because `ntotal == 0` makes `num_slots == 0` and an empty `codes` is then "valid". 3. `IndexPreTransform` deserialization skips all consistency checks when the chain is empty -- the `chain[0].d_in == ixpt->d` / `chain[nt-1].d_out == index->d` validation sits entirely inside `if (nt > 0)`. With `nt == 0`, `IndexPreTransform::reconstruct` passes the caller's buffer (sized from the outer `d`) straight to the sub-index, so a mismatched sub-index `d` is also a write-side overflow, for any sub-index type. Each is fixed: - `IndexFlatPanorama::reconstruct` / `reconstruct_n` now range-check against `ntotal`. - The `IxFP`/`IxFp` branch applies the same `d` and `ntotal` checks as `read_index_header`. - `read_index` requires `index->d == ixpt->d` when the `IxPT` chain is empty. Two adjacent gaps in the same audit are closed as well: - `IHfP` (`IndexHNSWFlatPanorama`) never validated `cum_sums.size()`, unlike the equivalent `IxFP` check. Both `get_cum_sum()` and `HNSW.cpp`'s Panorama search path index it at stride `pano.n_levels + 1`, so a short vector is an OOB read at search time. It also had no explicit `n_levels > 0` guard. - `ArrayInvertedListsPanorama::get_single_code` bounds-checked with `assert`, which is compiled out in opt builds. It reaches the same `Panorama::reconstruct` sink via `IndexIVFFlatPanorama::reconstruct_from_offset`. Reviewed By: mnorris11 Differential Revision: D118495504 fbshipit-source-id: 46bc7a1a6c9335a5589e20221b9478ae5505b9da |
||
|
lyang24
|
7f4cedda9c |
Add staged RaBitQ storage to HNSW (#5526)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5526 Reviewed By: junjieqi, mnorris11 Differential Revision: D116650696 Pulled By: alibeklfc fbshipit-source-id: 68ec701da84bd72a71e83ae5fadc041b2890eefe |
||
|
Binozo
|
2ed4c106e9 |
Fix: Windows SIMD detection (#5497)
Summary: Hi, while testing on windows x64 systems I've noticed that the `dd` optimization compilation target doesn't work properly. Causing crashes on non-avx2 systems for instance. This pr fixes the SIMD level detection. MSVC doesn't define `__x86_64__` but does define `_M_X64`. This is what this patch is based on. I got some nice [GitHub actions](https://github.com/Binozo/Faiss.NET/actions/runs/30251011773/job/89928775910) and tested the patch on it. ##### Based on https://github.com/facebookresearch/faiss/issues/5476 but reopened because I made a mistake while rebasing Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5497 Reviewed By: alibeklfc Differential Revision: D117897685 Pulled By: mnorris11 fbshipit-source-id: 80987895d17945bac9a4c5f0eb14beb67c84cbd5 |
||
|
kzahiri1
|
b4c66ba603 |
Fix reverse_index_factory/get_code_size round-trip for HNSW, IMI and IVFPQR (#5564)
Summary:
Three defects in `contrib/factory_tools.py` where a reversed factory string rebuilds a different index or fails to parse. `test_get_code_size_hnsw_roundtrip` already pins this invariant for flat HNSW; these cases escape it.
1. **HNSW storage dropped.** The `IndexHNSW` branch never read `index.storage`, so `HNSW32,SQ8` and `HNSW32,PQ16x8` both reversed to `"HNSW32"` and rebuilt as `IndexHNSWFlat`. `get_code_size` then applied the flat `d*4` formula: at d=128 it reported 768 bytes/vector for an index whose storage is 128 plus the 256-byte graph estimate, so 384.
2. **IMI regex fields transposed.** `index_factory.cpp:295` parses `IMI2x([0-9]+)`, but the regex read `IMI\d+x2`. Every real IMI key (`IMI2x10,PQ16`, and the ones in `demos/demo_auto_tune.py`) raised `RuntimeError("cannot parse")`.
3. **IndexIVFPQR matched by the IndexIVFPQ check**, dropping the refinement stage: `IVF64,PQ8+16` reversed to `IVF64,PQ8x8`, 8 bytes instead of 24.
Consumers are `benchs/bench_all_ivf/parse_bench_all_ivf.py` (imports `get_code_size` as `unitsize`) and `benchs/bench_fw/index.py`.
Flat storage stays implicit as the bare `HNSW<M>` string, and a null `storage` pointer still returns `HNSW<M>` instead of raising.
**Behavior change worth flagging:** `IndexHNSW2Level` storage is `Index2Layer`, which `reverse_index_factory` does not handle, so it now raises `NotImplementedError` instead of silently returning a wrong `"HNSW32"`. It is not reachable from `index_factory`.
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5564
Test Plan:
Four tests added to `TestFactoryTools`. All four fail on unpatched `factory_tools.py` and pass with it:
```
# pristine contrib/factory_tools.py, new tests present
4 failed, 7 passed
FAILED test_get_code_size_hnsw_non_flat_storage
FAILED test_get_code_size_imi
FAILED test_hnsw_storage_reverse_index_factory
FAILED test_ivfpqr_reverse_index_factory
# with the fix
11 passed
```
Full `tests/test_contrib.py tests/test_factory.py`, pristine vs branch:
```
pristine: 2 failed, 88 passed
branch: 2 failed, 92 passed
```
Failure sets are identical. The two are `TestComputeGT::test_compute_GT_gpu` and `test_compute_GT_ip_gpu`, pre-existing and unrelated (`AttributeError: module 'faiss' has no attribute 'GpuMultipleClonerOptions'` on a CPU-only build).
An A/B sweep over 40 factory strings changes exactly the 7 expected lines and nothing else:
```
- HNSW32,SQ8 IndexHNSWSQ HNSW32 768
+ HNSW32,SQ8 IndexHNSWSQ HNSW32,SQ8 384
- HNSW32,SQfp16 IndexHNSWSQ HNSW32 768
+ HNSW32,SQfp16 IndexHNSWSQ HNSW32,SQfp16 512
- HNSW32,PQ16x8 IndexHNSWPQ HNSW32 768
+ HNSW32,PQ16x8 IndexHNSWPQ HNSW32,PQ16x8 272
- IVF64,PQ8+16 IndexIVFPQR IVF64,PQ8x8 8
+ IVF64,PQ8+16 IndexIVFPQR IVF64,PQ8+16 24
- IMI2x5,Flat IndexIVFFlat IMI2x5,Flat <RuntimeError>
+ IMI2x5,Flat IndexIVFFlat IMI2x5,Flat 512
- IMI2x5,PQ8 IndexIVFPQ IMI2x5,PQ8x8 <RuntimeError>
+ IMI2x5,PQ8 IndexIVFPQ IMI2x5,PQ8x8 8
- IMI2x8,PQ8x8 IndexIVFPQ IMI2x8,PQ8x8 <RuntimeError>
+ IMI2x8,PQ8x8 IndexIVFPQ IMI2x8,PQ8x8 8
```
flake8 at the 80-char limit from CONTRIBUTING reports the same 12 pre-existing findings before and after, none in the changed lines.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Reviewed By: mnorris11
Differential Revision: D118661065
Pulled By: trang-nm-nguyen
fbshipit-source-id: b50a9522361e3fd14f276b36196d3c728567b980
|
||
|
ethanglaser
|
cfbf4ec5ff |
SVS: allow option to disable vector storing (#5525)
Summary: Allow avoiding storing original vectors, which may not necessarily be used, in which case they would be wasting memory Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5525 Reviewed By: alibeklfc Differential Revision: D118722620 Pulled By: mnorris11 fbshipit-source-id: 9757fa3d272187308c750fb6c6d0e3a6e8ab2bb3 |
||
|
Aalekh Patel
|
cda4f6e713 |
Expose IndexBinaryIVF.set_direct_map_type via the C API (#5451)
Summary: Expose the `set_direct_map_type` method of binary IVF indexes via the C API. This enables support for the `index.set_direct_map_type` API in languages other than Python. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5451 Reviewed By: alibeklfc Differential Revision: D114681970 Pulled By: mnorris11 fbshipit-source-id: 1342eeb225d6fd9fdea76c6b9417cedde55c1d3a |
||
|
generatedunixname2238252550246476
|
c0084f5cd2 |
Clamp EDEN unbiased L2 distances to be non-negative (#5569)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5569 https://www.internalfb.com/monitoring/oncall_operator?board=1003685189388103&workItem=4614322948811275 --- `IndexEDEN` / `IndexIVFEDEN` with the unbiased 1-bit EDEN scale can return negative `METRIC_L2` distances, violating the contract that L2 distances are never negative. Root cause: for the unbiased scale, `distance_from_code_dot` in `faiss/impl/scalar_quantizer/EDENQuantizer.cpp` reports `query_base + l2_norm_term - 2*scale*<q_code, x_res>`, where the encoder sets `l2_norm_term = ||r||^2` (the *original* residual norm) rather than the reconstructed-code norm `scale^2 * ||q_code||^2`. Unlike the biased scale (where the three terms complete the square `||x_res - r_y||^2 >= 0`), the unbiased branch is an unbiased *estimator* of the squared distance, so for a query residual that aligns strongly with the code direction the subtracted term can exceed `query_base + ||r||^2` and the result goes negative. Every distance path (reference, LUT, and the AVX2/AVX-512 batch variants) routes through `distance_from_code_dot`, and none of them clamped the result. Fix: clamp the L2 branch of `distance_from_code_dot` to zero. This is the single choke point, so it covers every distance and search path. The clamp only floors estimates that were already below the true minimum of zero. Design note for reviewers: clamping biases the unbiased estimator upward for the very closest matches (whose estimates are the ones that dip below zero), and collapses their fine ordering at exactly zero. The alternative — leaking negative squared distances to callers that take a sqrt or compare against a radius — is worse, and this is the most reversible fix. If the team prefers to preserve the strict unbiased estimator and instead relax the non-negativity expectation, that is a larger design decision to make separately. Reviewed By: trang-nm-nguyen Differential Revision: D118611745 fbshipit-source-id: 8c3d6a279fb15a92697589909f89eca218915e52 |
||
|
Trang Nguyen
|
6457e941f5 |
Contain DirectoryArchiver entries within the extraction root (#5561)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5561 `svs::lib::DirectoryArchiver::read_file` builds its output path as `root / filename`, where `filename` is read straight off the deserialization stream. Neither guard on that path contains it: - `std::filesystem::operator/` **discards `root` entirely** when the right operand is absolute, so `filename = "/etc/cron.d/x"` yields `path = "/etc/cron.d/x"`. - A `..` component climbs out of `root` normally. - `check_file(path, out|binary)` only verifies that `path.parent_path()` is an existing directory. It performs no containment check — and that check is satisfied by construction anyway, because `read_file` calls `fs::create_directories(parent_dir)` two lines earlier. The result is that `faiss::read_index_up()` on untrusted bytes can create or overwrite an arbitrary file with attacker-controlled content, bounded only by the process's filesystem permissions. The harness manifest already rates this surface `REMOTE` / `PRE_AUTH` / `ZERO_CLICK`. This is not theoretical. The new tests fail on unpatched code with `std::filesystem::exists(probe)` returning **true** — the file really is written outside the temp root. Note the deserialization then fails with an unrelated `"File could not be opened for reading"`, so the write is a silent side effect that never surfaces as an error to the caller. The fix normalizes both sides and rejects any entry whose resolved path is not under `root`, before `create_directories` runs so that a rejected entry leaves nothing behind. Legitimate entries are unaffected: plain names, nested `sub/dir/name`, `./name`, and interior `..` that stays inside `root` all still resolve normally — only escapes are rejected. `third-party/svs/v0.4.0` is pristine vendored code (`c5e5dbb35bbf`, never patched internally), so this also needs reporting upstream to Intel via their vulnerability handling process (`src/SECURITY.md`) rather than as a public PR, so it survives the next version bump. The related unbounded-allocation defect in `Archiver::read_name`, on the same call path, is fixed separately in the next diff in this stack (T287110736). Reviewed By: mnorris11 Differential Revision: D118494326 fbshipit-source-id: 7f7851111375eda831e4a7dac0acb7cdd1f80dc8 |
||
|
Tamim Ehsan
|
ac60182fc0 |
RaBitQ: Fix zero residual sign convention (#5553)
Summary: The RaBitQ 1 bit quantization, code is quantized comparing positive vs negative or zero https://github.com/facebookresearch/faiss/blob/718002f7cf269f3f3752dde04a4cc3f8eabb0050/faiss/impl/RaBitQuantizer.cpp#L147-L154 However, during ex bit quantization, the signs are flipped under condition negative vs positive or zero. https://github.com/facebookresearch/faiss/blob/718002f7cf269f3f3752dde04a4cc3f8eabb0050/faiss/impl/RaBitQuantizerMultiBit.cpp#L305-L310 Alternative implementation can be taking the equality in the one bit path. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5553 Reviewed By: mnorris11 Differential Revision: D118529759 Pulled By: alibeklfc fbshipit-source-id: 319f0ec06952c73616acac25c37c63dfa7f9203c |
||
|
generatedunixname26860585020226206
|
1a0cfa64e0 |
Apply the deserialization byte limit to SVS IVF (#5568)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5568 `read_index` can cap the bytes one read pulls from an untrusted index file (`set_deserialization_vector_byte_limit`). Three of the five SVS deserialization sites pass it down; the two SVS IVF sites do not, so it is ignored on every SVS IVF index. Impact: a corrupt or hostile IVF file can drive an unbounded read into the third-party SVS reader. The faiss fuzzers set this limit to stop that. Fix: pass `get_deserialization_vector_byte_limit()` at both sites, matching Flat and Vamana. Sentinel-Council-Run: council_1788372198_15590275 Sentinel-Harness: claude *Modify your team agent prompt, check stats, and leave feedback: https://www.internalfb.com/sentinel_agent/rotations/faiss* Model used: Claude Opus 5 (1M context), claude-opus-5[1m] Reviewed By: trang-nm-nguyen Differential Revision: D118524339 fbshipit-source-id: 4ed77fc6cd6a1894a1846cd427d299bd1c677f15 |
||
|
marcelo-cjl
|
256980cb6e |
Add SVE low-dimensional nearest fast path and spherical SuperKMeans support (#5530)
Summary:
This PR adds two related changes for the SCANN coarse quantizer / PQ encoding path on ARM SVE: a low-dimensional nearest-neighbor fast path and spherical SuperKMeans support.
### 1. Low-dimensional L2sqr nearest fast path (d in {2,4,8})
`fvec_L2sqr_ny_nearest<ARM_SVE>` previously wrote all `ny` distances to the scratch buffer and then performed a separate linear scan. For PQ encoding (`compute_1_code`) each call has `d = dsub` and `ny = ksub`, making this the hot path. The new path maps one centroid to each SVE lane, gathers its `D` components, and keeps lane-local minimum distances and ids across batches. A predicated tail handles non-multiple `ny` values, equal distances preserve the first index, and only one final horizontal reduction is needed. The scratch buffer is not written, matching the x86 AVX2/AVX512 `D2/D4/D8` implementations.
`ProductQuantizer::compute_code` already uses the A1 dispatch on current `main`, so the ARM_SVE specialization remains reachable from PQ encoding after this rebase.
### 2. SuperKMeans spherical (inner-product) support
SuperKMeans previously only supported L2 clustering; the header explicitly excluded IP/cosine. With `cp.spherical=true`:
- `TrainState::R` becomes `std::unique_ptr<VectorTransform>`; power-of-two `d` uses the fast `HadamardRotation`, L2 keeps `RandomRotationMatrix`.
- Forgy initialization produces unit centroids before the first assignment. Each iteration computes and splits centroids first, then applies spherical centroid post-processing once, matching the `Clustering` flow.
- `HadamardRotation::reverse_transform` fills the missing inverse.
- `ClusteringParameters::use_super_kmeans` (default false) lets `Level1Quantizer::train_q1` route coarse quantizer training through SuperKMeans when explicitly enabled; the Python type stubs expose the new option.
- `block_l2<ARM_SVE>` completes the SuperKMeans SIMD kernels for the ADSampling pruning loop.
## Tests
- Low-dim nearest across SIMD levels: `d` in `{2,4,8}` and `ny` around common SIMD widths through 257, including non-multiple tails and first-index tie behavior. The dispatch test source is included in DD builds.
- Spherical objective close to vanilla spherical Clustering; unit-norm centroids.
- `use_super_kmeans` field inherited from `ClusteringParameters`.
## Benchmark
qwen 4096-dim, IP, nlist=1024, sub_dim=4, 1 thread, 50k rows:
| stage | baseline | optimized | change |
| --- | ---: | ---: | --- |
| Train | 67.73s | 24.36s | -64% |
| Add | 10.86s | 7.17s | -34% |
| Build | 78.59s | 31.52s | -60% |
Train speedup comes from SuperKMeans spherical coarse quantizer training; Add speedup comes from the low-dim SVE nearest fast path in PQ encoding. recall@10 unchanged vs Clustering baseline (diff <= 0.0002).
## Notes
This change targets ARM SVE. The x86 (AVX2/AVX512) implementations already have equivalent low-dim nearest specializations. The generic ARM SVE distance path comes from current `main`; this PR adds the low-dimensional nearest specializations on top of it.
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5530
Reviewed By: mnorris11
Differential Revision: D117563661
Pulled By: alibeklfc
fbshipit-source-id: 47ab1064d781a45d009923b4d5e62525718e83bf
|
||
|
Trang Nguyen
|
718002f7cf |
Fix distance conversion in IndexBinaryFromFloat for non-L2 metrics (#5549)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5549 `IndexBinaryFromFloat::search()` converted the wrapped float index's distances back to Hamming distances with a hardcoded `round(df[i] / 4.0)`, which is only correct for `METRIC_L2`. For any other metric the returned distances were silently wrong. `binary_to_real()` maps bits to `{-1, +1}`, so the relation between the float distance and the Hamming distance depends on the metric: - `METRIC_L2` (squared): `float_dist = 4 * hamming` - `METRIC_INNER_PRODUCT`: `float_dist = d - 2 * hamming` - `METRIC_L1`: `float_dist = 2 * hamming` Branch on `index->metric_type` and apply the matching inverse. `METRIC_L2` remains the default/fallback, so existing behavior is unchanged. Each mapping is monotonic in the Hamming distance, so the result order the wrapped index produces is already correct and needs no reordering. Reported in https://github.com/facebookresearch/faiss/issues/5546. ___ Differential Revision: D118160087 fbshipit-source-id: f2ce2b97ebfd47461346508f9878b1af7e321b12 |
||
|
generatedunixname1395027625275998
|
af9eeffcf7 |
Reserve cs vector capacity in hammings_knn_mc_impl to avoid reallocations (#5548)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5548 Reviewed By: rjaber Differential Revision: D118055043 fbshipit-source-id: 47f6ecdf6197610ccbcd60a466a3bfc77cfcc4e1 |
||
|
Chao Zhang
|
3a716ead1a |
Add Windows ARM64 support to CI (#5544)
Summary: Hi team, I'm from Microsoft and recently I'm working on improving Python ecosystem support for Windows on Arm. So I updates the CI workflows to add support for Windows ARM64 build and release. Could you please help to review? Thanks. **Windows ARM64 support and OpenBLAS installation improvements:** * Added a new PowerShell script `install-openblas.ps1` that automatically detects the runner architecture (x64 or ARM64), downloads the appropriate OpenBLAS binary, verifies its hash, and installs it to `C:\openblas`. This ensures consistent and secure OpenBLAS setup on all Windows runners. * Updated the build matrix in `.github/workflows/build-pip.yml` to include `windows-11-arm`, enabling CI builds and tests on Windows ARM64 runners. * Refactored the Windows `before-all` build step in `pyproject.toml` to use the new PowerShell script, replacing the previous hardcoded x64-only download command. This allows seamless support for both x64 and ARM64 architectures. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5544 Reviewed By: mnorris11 Differential Revision: D117818430 Pulled By: trang-nm-nguyen fbshipit-source-id: dac342878d063828437952e1b19f76e6699e1b6b |
||
|
mayuriphad
|
02ea14372c |
Fix range_search_max_results to respect all similarity metrics (#5533)
Summary: ## Bug `range_search_max_results()` (and its helper `apply_maxres()`) in `contrib/exhaustive_search.py` decide whether to keep the highest- or lowest-scoring results when pruning the accumulated results down to `max_results`, based on: ```python keep_max=index.metric_type == faiss.METRIC_INNER_PRODUCT ``` This hardcodes the assumption that `METRIC_INNER_PRODUCT` is the only "similarity" metric (higher = better). But `faiss.is_similarity_metric()` (see `faiss/MetricType.h`) also treats `METRIC_Jaccard` as a similarity metric, and this same, correct check is already used a few lines above in `range_search_gpu()` in the very same file (`keep_max = faiss.is_similarity_metric(index_gpu.metric_type)`). As a result, when `range_search_max_results` is used with a `METRIC_Jaccard` index and the result set grows past `max_results`, the pruning logic incorrectly discards the *highest*-similarity matches and keeps the *lowest*-similarity ones, and moves the returned radius in the wrong direction. ## Root cause Two call sites in `contrib/exhaustive_search.py` compare `index.metric_type` directly against `faiss.METRIC_INNER_PRODUCT` instead of using the library's own `faiss.is_similarity_metric()` helper, which is the single source of truth for which metrics are "larger is better". ## Fix Replace both occurrences of `index.metric_type == faiss.METRIC_INNER_PRODUCT` with `faiss.is_similarity_metric(index.metric_type)`, matching the existing pattern used in `range_search_gpu` in the same module. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5533 Test Plan: Added `test_query_iterator_similarity_metric` to `tests/test_contrib.py`. It exercises `range_search_max_results` against a lightweight fake index using `METRIC_Jaccard` (real `IndexFlat.range_search` doesn't support `METRIC_Jaccard` directly, so a fake index is used to isolate the pruning logic under test) and asserts that pruning keeps the highest-scoring results and raises the radius accordingly. - Confirmed the new test fails on `main` (old code keeps the *lowest*-similarity results and lowers the radius) and passes after the fix, by swapping the module in-place against an installed `faiss-cpu` and running the assertions directly (full repo `pytest` collection isn't possible in this environment without rebuilding the C++ extension for `main`, since the installed wheel is older and `tests/common_faiss_tests.py` on `main` references APIs from a newer build). - The fix is a pure two-line change that only affects the boolean passed to `apply_maxres`, verified by direct comparison of before/after behavior with the reproduction script. Reviewed By: mnorris11 Differential Revision: D117001619 Pulled By: alibeklfc fbshipit-source-id: 27e6757951d63a4d4f4f998640a1726ef8f6c220 |
||
|
Jeremy Schoemaker
|
bd7087453e |
fix(faiss): use is_similarity_metric for IndexShardsIVF merge (#5541)
Summary: IndexShardsIVF merges non-L2 distance metrics as similarities, reversing L1 and other metrics at IndexShardsIVF.cpp:230. The search method checked only METRIC_L2 for CMin and used CMax for everything else. Metrics such as L1, Linf, Lp, Canberra and BrayCurtis are distances and require CMin. Only inner product and Jaccard are similarities. Fix: replace the METRIC_L2 check with !is_similarity_metric(metric_type) so all distance metrics use CMin and only similarities use CMax. Add the MetricType header for the helper. Verified by syntax check and inspecting merge logic. The diff is minimal and does not change formatting elsewhere. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5541 Reviewed By: mnorris11 Differential Revision: D117542369 Pulled By: juancarpio27 fbshipit-source-id: 8cab4275cc9868c46100bc60ed14349544fd2aab |
||
|
William
|
561e6ecdf3 |
Merge shard results by metric type rather than assuming similarity (#5509)
Summary:
`IndexShardsTemplate::search` picked the comparator for merging per-shard results by testing for `METRIC_L2`:
```cpp
if (this->metric_type == METRIC_L2) {
merge_knn_results<idx_t, CMin<distance_t, int>>(...); // smaller is better
} else {
merge_knn_results<idx_t, CMax<distance_t, int>>(...); // larger is better
}
```
Everything that is not `METRIC_L2` was therefore merged as a similarity. `METRIC_L1`, `METRIC_Linf` and `METRIC_Lp` return distances, so once results crossed a shard boundary they came back farthest-first, while the same vectors in a single unsharded index came back nearest-first.
This PR selects the comparator with the existing `is_similarity_metric()` helper from `MetricType.h`, which is exactly the distinction the branch needs.
Fixes https://github.com/facebookresearch/faiss/issues/5503
## Behaviour change
`is_similarity_metric()` is true only for `METRIC_INNER_PRODUCT` and `METRIC_Jaccard`, so:
| metric | before | after |
| --- | --- | --- |
| `METRIC_L2` | `CMin` | `CMin` (unchanged) |
| `METRIC_INNER_PRODUCT` | `CMax` | `CMax` (unchanged) |
| `METRIC_L1`, `METRIC_Linf`, `METRIC_Lp`, `METRIC_Canberra`, `METRIC_BrayCurtis`, `METRIC_JensenShannon` | `CMax` | **`CMin`** |
| `METRIC_Jaccard` | `CMax` | `CMax` (unchanged) |
`IndexShardsTemplate<IndexBinary>` is also unaffected: `IndexBinary::metric_type` defaults to `METRIC_L2`, which keeps `CMin` as before.
## Reproduction
Against the released `faiss-cpu` 1.15.0 wheel, using the reporter's example:
```python
import faiss, numpy as np
xb0 = np.array([[0., 0.]], dtype='float32')
xb1 = np.array([[10., 0.]], dtype='float32')
xq = np.array([[1., 0.]], dtype='float32')
flat = faiss.IndexFlat(2, faiss.METRIC_L1)
flat.add(np.vstack([xb0, xb1]))
print(flat.search(xq, 2))
s0 = faiss.IndexFlat(2, faiss.METRIC_L1); s0.add(xb0)
s1 = faiss.IndexFlat(2, faiss.METRIC_L1); s1.add(xb1)
shards = faiss.IndexShards(2, False, True)
shards.add_shard(s0); shards.add_shard(s1)
print(shards.search(xq, 2))
```
```
single IndexFlat D,I = [[1. 9.]] [[0 1]] # nearest first, correct
IndexShards D,I = [[9. 1.]] [[1 0]] # farthest first, reversed
```
## Test
Adds `Shards::test_shards_distance_metric_ordering` to `tests/test_meta_index.py`. It splits the dataset across three `METRIC_L1` shards and checks that each result row is ordered nearest-first and matches the distances an unsharded `IndexFlat(METRIC_L1)` returns. Distances rather than labels are compared so that equidistant neighbours may be returned in either order.
The test fails on `main` (the rows come back reversed) and passes with this change.
## Testing notes
I reproduced the bug against the released 1.15.0 wheel as shown above, but I was not able to build faiss from source on this machine (Windows, no local C++ toolchain), so I have not executed the C++ build or run the test suite locally. The added test's pass/fail claim above follows from the comparator change rather than from a local run. Please treat CI as the gate, and I am happy to adjust if anything in the suite disagrees.
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5509
Reviewed By: juancarpio27
Differential Revision: D117536893
Pulled By: mnorris11
fbshipit-source-id: 8568437eb596a202262fc455748063fafca202c1
|
||
|
Fu Xiaonan
|
613e0acac5 |
Fix IndexIDMap range search parameters (#5536)
Summary: - preserve the concrete search-parameter type when `IndexIDMap::range_search` translates an external-ID selector - retain IVF-specific per-query fields such as `nprobe` by temporarily replacing only `sel`, using the same scoped restoration pattern as `search_ex` - add a deterministic regression test with two IVF lists and external IDs ## Testing - confirmed the new test fails against FAISS 1.14.2 with `IndexIVF params have incorrect type` - built the current source on macOS arm64 with Python bindings (`FAISS_OPT_LEVEL=generic`, GPU and Metal disabled) - `python -m pytest -q tests/test_search_params.py` (51 passed) - `clang-format 21 --dry-run --Werror faiss/IndexIDMap.cpp` - `git diff --check` Closes https://github.com/facebookresearch/faiss/issues/5523 Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5536 Reviewed By: mnorris11 Differential Revision: D117233222 Pulled By: juancarpio27 fbshipit-source-id: 28e750f63fb681cc915ad41443c582739d8c3597 |
||
|
Juan Carpio Castellanos
|
8b01f308f0 |
Validate IndexScalarQuantizer codes buffer size on deserialization (#5540)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5540 When reading a serialized ScalarQuantizer-based flat index, the codes buffer length was not checked against ntotal * code_size, unlike the equivalent read paths for other flat-codes index types. A crafted index file with an inflated ntotal but a short codes buffer causes out-of-bounds reads during decode/reconstruct. Add the same size check used by the other index types right after the codes buffer is read. ___ Differential Revision: D117319393 fbshipit-source-id: 70dd1f09c8daf7393b992908fa808b111d0a02a1 |
||
|
ThatikondV
|
317a207bca |
ARM SVE optimization for L2 distance kernels adapted to new simd_impl structure (#5098)
Summary: This PR introduces ARM SVE (Scalable Vector Extension) optimization for L2 metric functions. The new implementation extends the existing NEON/SVE SIMD logic with SVE vectorized kernels for distance computations, improving both indexing and search performance. **Benchmarking Setup:** To evaluate the performance impact of the new SVE-enabled L2 distance computations, a dedicated benchmarking script was developed. This script systematically compares the earlier FAISS version and the latest SVE-optimized version using the SIFT1M dataset with 128-dimensional vectors. The benchmarks cover all major index types like FLAT, IVF_FLAT, IVF_PQ, IVF_SQ8, and HNSW. **Results:** Index | OSS QPS | SVE QPS | Improvement -- | -- | -- | -- Flat (L2) | 888.53 | 1188.12 | 33.70% IVF1024-Flat | 33,437.76 | 45,871.00 | 37.20% IVF1024-PQ32x8 | 33,597.01 | 40,650.00 | 21.00% HNSW (M=16, ef=64) | 72,837.67 | 77,482.00 | 6.40% IVF1024-SQ8 | 17,002.79 | 20,657.00 | 21.50% The above results clearly demonstrate that enabling SVE vectorization yields consistent performance gains across all tested index types, where both indexing and search times improved significantly. These improvements validate the effectiveness of the SVE implementation in accelerating L2 distance computations on modern ARM platforms. **Co-author:** This work was jointly developed with abhijain1204fujitsu. Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5098 Test Plan: Run on an aarch64 host with SVE2, `svcntw() == 4` (128-bit vectors). **The SVE code only exists in dynamic-dispatch builds.** In the default static aarch64 Buck build, `xplat.bzl` filters `utils/simd_impl/distances_arm_sve.cpp` out of `srcs` and `SINGLE_SIMD_LEVEL` resolves to `ARM_NEON`, so a static-mode run exercises nothing here. CI runs both modes. ## Correctness ``` buck2 test -c faiss.dynamic_dispatch=true \ fbcode//faiss/tests:test_distances_simd \ fbcode//faiss/tests:test_distances_dispatch \ fbcode//faiss/tests:test_pq_encoding \ fbcode//faiss/tests:test_ivfpq_indexing \ fbcode//faiss/tests:test_ivfpq_codec \ fbcode//faiss/tests:test_pq_code_distance \ fbcode//faiss/tests:test_disable_pq_sdc_tables # Pass 32. Fail 0. buck2 test -c faiss.dynamic_dispatch=true \ fbcode//faiss/tests:test_build_blocks fbcode//faiss/tests:test_product_quantizer # Pass 105. Fail 0. buck2 test -c faiss.dynamic_dispatch=true fbcode//faiss/tests:test_index # Pass buck2 test fbcode//faiss/tests:test_distances_simd fbcode//faiss/tests:test_distances_dispatch # static mode: Pass 9. Skip 6 (only one SIMD level available). ``` `buck2 build fbcode//mode/opt fbcode//faiss:faiss -c faiss.dynamic_dispatch=true` and the static build both clean. `arc f` / `arc lint` clean. `test_distances_dispatch` forces every available level and compares against `SIMDLevel::NONE`; in DD mode on this host it reports `NONE`/`ARM_NEON`/`ARM_SVE` and no longer skips, so the `ARM_SVE` specializations are genuinely under test. Added coverage in `tests/test_distances_simd.cpp`: - `TestFvecL2sqrNy.D2`: `dim` swept over `{1,2,3,4,5,8,12,16,24,32,48,64}` (was `{2,4,8,12}`) so the `lanes1`..`lanes4` kernels are reached on 128/256/512-bit SVE, not just the `d1/d2/d4/d8` cases. - Both `_y_transposed` tests: `ny` swept over `{5,8,11,16,32}` (was hardcoded `11`, which never produced a full-vector block on any SVE width, so the no-tail path was untested). ## Kernel microbenchmark `mode/opt`, DD, pinned core, alternating runs, min-of-N x median-of-7, run-to-run spread <1%. `ny = 256` (ksub), `d = dsub`. vs this diff's parent: | kernel | speedup range over parent | |---|---| | `fvec_L2sqr_ny` | +17% to +725% (dsub 3 and 6 hit the scalar fallback, unchanged) | | `fvec_L2sqr_ny_nearest` | +48% to +130% | | `fvec_L2sqr_ny_transposed` | +234% to +298% | | `fvec_L2sqr_ny_nearest_y_transposed` | +318% to +658% | ## Index-level `faiss.contrib.datasets.SyntheticDataset`, d=128, nb=400k, nq=2k, k=1, single-threaded, min/best of 3 alternating rounds. "kernels" = SVE kernels with the pre-existing A0 dispatch; "wired" = this diff. PQ encode, `add()` seconds: | Index | parent | kernels only | wired | speedup | |---|---|---|---|---| | IVF1024-PQ32x8 | 16.17 | 16.16 | 6.32 | **2.56x** | | PQ32x8 (flat) | 14.80 | 14.82 | 4.97 | **2.98x** | | Flat / IVF-Flat / HNSW / IVF-SQ8 | - | - | - | 1.00x | Search QPS is unchanged everywhere (-0.9% to +0.5%, inside 0.2-3.0% run-to-run variance). recall@1 is identical to the parent for every index. The "kernels only" column is 1.00x because `ProductQuantizer` dispatched through `with_simd_level()` = `AVAILABLE_SIMD_LEVELS_A0`, which lacks the `ARM_SVE` bit, so the `case SIMDLevel::ARM_SVE:` arm failed its `if constexpr` and fell through to `ARM_NEON`. Verified by putting `abort()` in `fvec_L2sqr_ny_nearest<ARM_SVE>`: PQ encoding completed without hitting it, while `test_distances_dispatch` (A1) aborted immediately. The three `with_simd_level_a1` call sites in `impl/ProductQuantizer.cpp` are what close that gap. `compute_inner_prod_table` still uses A0 and so cannot reach the pre-existing `fvec_inner_products_ny<ARM_SVE>`; left alone as it is outside this diff's L2 scope and unmeasured here. Reviewed By: junjieqi Differential Revision: D106149015 Pulled By: mnorris11 fbshipit-source-id: c1144fc8fd0eefb30c9dcc8084b472152623a88e |
||
|
tandede
|
2832ac3f67 |
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 |
||
|
Michael Norris
|
e135a1a667 |
Fix OSX arm64 nightly: bump libopenblas from 0.3.33 to 0.3.34 (#5538)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5538 The `OSX arm64 nightlies` job has been failing to solve the `libfaiss` environment since D115218647: ``` ExplainedDependencyNeedsBuildingError: Unsatisfiable dependencies for platform osx-arm64: {MatchSpec("openblas==0.3.34=openmp_hea878ba_0")} - package libopenblas-0.3.33-pthreads_hddb8425_0 has constraint openblas >=0.3.33,<0.3.34.0a0 conflicting with openblas-0.3.34-openmp_hea878ba_0 ``` `conda/faiss/meta.yaml` pins both outputs of the OpenBLAS feedstock, and on osx-arm64 both selectors apply at once: `openblas =0.3.34 # [not x86_64]` and `libopenblas =0.3.33 # [osx]`. Since `libopenblas 0.3.33` constrains `openblas >=0.3.33,<0.3.34.0a0`, the two specs are mutually exclusive and the recipe can never solve. D115218647 bumped `openblas` to 0.3.34 to get `Linux arm64 (conda)` green and left the osx `libopenblas` pins at 0.3.33 on the grounds that that package was resolving fine. It was — in isolation. But the two pins are version-coupled, so the bump broke Mac. The pins also land on different platforms, which is why only OSX fails: `# [not x86_64]` gives linux-arm64 the `openblas` pin on its own. Bump the six osx `libopenblas` pins to 0.3.34 so both feedstock outputs agree. This matches how the same breakage was fixed the last time `openblas` moved: D98293081 bumped `openblas` 0.3.30 to 0.3.32 and broke this job, D98718117 fixed it by bumping `libopenblas` to match, and D104902441 moved both together when going 0.3.32 to 0.3.33 and stayed green. Reverting `openblas` to 0.3.33 is not an option here — it would re-break `linux-arm64-nightly`. The GPU recipes need no change: they carry no osx `libopenblas` pins, since GPU builds are Linux-only. Reviewed By: junjieqi Differential Revision: D117300512 fbshipit-source-id: fe55ef37f4a3054689eacb2f7f64a21ff1d1e8b2 |
||
|
lyang24
|
7059eaf7da |
Add AVX512 bitplane kernel for multi-bit RaBitQ inner product (#5522)
Summary:
`compute_inner_product<AVX512>` only had a dedicated kernel for `ex_bits == 1`; every wider configuration fell through to `ip_bitplane_avx2`, so AVX512 builds ran 256-bit code for the RaBitQ widths used in practice (`nb_bits` 4-5). This adds `ip_bitplane_avx512`: a bitplane is already a bitmask, so it goes straight into a mask register and is consumed by a single masked add instead of the AVX2 broadcast/and/cmpgt/and/fmadd sequence, which also takes the GPR->vector broadcast off the dependency chain; and it processes 16 dimensions per iteration instead of 8. `_pext_u64` still runs once per 8 dims per plane, so only the vector side gets cheaper, and the memory access pattern is unchanged. The now-unreachable copy of `ip_bitplane_avx2` in this translation unit is removed; the AVX2 translation unit keeps its own. Top-10 labels are unchanged across `nb_bits` in {2,3,4,5,7,8} and both metrics, with distance deviation at the level expected from reassociating the accumulation from 8 to 16 lanes; `nb_bits == 2` stays bit-identical since that path is untouched.
== Benchmark ==
We measured `compute_inner_product<AVX512>` directly, A/B against this diff's base revision. Same host, same binary target, `mode/opt`, dynamic dispatch enabled, 5 repetitions, medians reported. Speedup is base time divided by diff time.
| d | `ex_bits` 2 | `ex_bits` 3 | `ex_bits` 4 | `ex_bits` 7 |
| --- | --- | --- | --- | --- |
| 128 | 1.19x | 1.18x | 1.10x | 1.04x |
| 136 | 1.15x | 1.14x | 1.07x | 1.02x |
| 768 | 1.33x | 1.23x | 1.12x | 1.00x |
| 1024 | 1.33x | 1.26x | 1.13x | 1.00x |
1. The production widths (`nb_bits` 4-5, i.e. `ex_bits` 3-4) gain **1.07x to 1.26x**. Best case overall is **1.33x** at `ex_bits` 2 and large `d`.
2. The gain decays to nothing by `ex_bits` 7 (1.00x-1.04x). `_pext_u64` work per plane is unchanged by this diff, so at wide `ex_bits` the GPR side dominates and the vector saving is no longer measurable.
3. Speedup grows with `d`, because the per-call `v_weights` and `pext_masks` setup is a fixed cost amortized over more dimensions.
4. `d = 136` runs 3-4% below `d = 128` at every width. That is the half-width cleanup step paying full 512-bit execution cost for 8 lanes of useful work.
Validity: `compute_inner_product<NONE>` was benchmarked in the same binaries as a control. Its code is unchanged by this diff and reads 1.00x in 15 of 16 cells, putting the noise floor at 0.1%-1.9%. The single exception (`d = 768`, `ex_bits` 2, 0.84x) made the diff side look slower, so it does not inflate any reported gain. AVX512 runs 3x-4x faster than scalar in both builds, confirming the dispatched path is the one being measured. These are kernel-level numbers; end-to-end index search gains will be smaller.
The harness was a temporary microbenchmark calling the kernel directly and is not included in this diff.
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5522
Reviewed By: mnorris11
Differential Revision: D116356583
Pulled By: alibeklfc
fbshipit-source-id: 388bae1e564e70f7136bfd595ff276e05ca9e25f
|
||
|
Michael Norris
|
e2cf9e1ebf |
Accept ragged code sizes in faiss::hammings(), and fix the vectorization axis for wide codes (#5520)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5520 Two changes to `faiss::hammings()`: it accepts ragged code sizes, and the kernel for wide codes is told which axis to vectorize. Throughout, **ncodes** is number of bytes per vector (so, 32 for a 256-bit vector), a **word** is one 64-bit (8-byte) unit. A code size is **word-multiple** when `ncodes % 8 == 0` and **ragged** otherwise, so a code is `nwords = ncodes / 8` whole words followed by `rem = ncodes % 8` trailing bytes. ## 1. "Ragged" code sizes Ragged here doesn't mean variable vector lengths, it means non-divisible by 8 ncodes. `hammings()` throws unless the code size is word-multiple, even though the other Hamming entry points (`hammings_knn_hc`, `hamming_range_search`) have always accepted ragged sizes. Callers who need them have to reach past `hammings()` into `with_simd_level_a0_spr` and `with_HammingComputer` and hand-roll the sweep; Laser does exactly that in `KnnFaissBinaryIndex::measureAllCentroids` (D115840549) and can drop it after this. Ragged sizes go to `hammings_ragged_dispatch()`, behind its own `hammings_ragged_fixSL<SL>` entry point so the word-multiple kernels keep the code they generate today. ## 2. Vectorization axis for wide codes All-pairs Hamming is a doubly nested loop: over candidates, and over the words of one code. Only one of them gets vectorized, and clang picks the candidate loop. That means each vector lane wants word `k` of a different candidate, and since candidates are `ncodes` bytes apart the load becomes a strided gather: ``` across candidates (today) within one code (this diff) vpgatherqq (%r8,%zmm19,1),%zmm20 vpxorq (%rbx,%r12,1),%zmm4,%zmm5 ``` Vectorizing the other loop instead reads one whole code contiguously, which is a plain vector load. `#pragma clang loop vectorize(disable)` on the candidate loop is what expresses that: it does not disable vectorization, it takes the candidate loop out of the running so the word loop is what gets vectorized. The arithmetic is identical either way. It is applied only when `nwords >= 4`, i.e. once a code is at least a full 256-bit register wide. That is the widest threshold that costs nothing anywhere: | threshold | ncodes 8 | ncodes 16 | ncodes 32 | ncodes 64 | | --- | --- | --- | --- | --- | | not applied | +0.0 / -0.1 | +0.0 / -0.6 | +0.1 / -0.2 | -0.1 / +0.2 | | `nwords >= 1` | **+45.3 / +63.5** | -10.4 / **+12.2** | -12.6 / -22.7 | -40.7 / -63.1 | | `nwords >= 2` | +0.1 / +0.0 | -10.5 / **+11.7** | -12.6 / -23.7 | -40.7 / -63.1 | | `nwords >= 4` | +0.2 / +0.1 | -0.1 / +0.1 | -12.7 / -23.7 | -40.7 / -63.1 | (AVX2 / AVX512, percent against the parent commit.) At one word the candidate loop is already contiguous, so taking it out of the running costs 45-63%. At two words it helps AVX2 at ncodes 16 and hurts AVX512 by the same amount, which would make the behaviour SIMD-level dependent. Four words is where both levels agree. The pragma is clang-only and guarded with `#if defined(__clang__)`, since MSVC warns C4068 on an unknown pragma. Other compilers therefore never see it and keep today's behaviour: correct, just without the gain. ## 3. Why the ragged kernel is templated A ragged code's loads must be `memcpy` because it is not 8-byte aligned. Both `nwords` and `rem` are template parameters, which is worth an order of magnitude: | ncodes | `<NW, REM>` | `<NW>`, rem runtime | neither templated | | --- | --- | --- | --- | | 2 | **0.487** | 8.720 | 8.724 | | 4 | **0.433** | 9.419 | 9.164 | | 10 | **0.868** | 9.344 | 10.160 | | 12 | **0.692** | 10.387 | 11.472 | | 20 | **1.066** | 9.879 | 10.730 | | 33 | **2.646** | 10.757 | 10.769 | Fig 1: ragged table comparing templates NW and REM (AVX2, ns per pair.) `rem` is the critical part: at a constant size `memcpy` folds into a single instruction, whereas at a runtime size it becomes a real `memcpy` call per candidate, costing 10-20x. That is why the dispatch switches on `ncodes % 8` rather than passing the remainder as an argument. `NW` is worth a further ~2x, but only for 1 to 3 whole words, where a compile-time trip count lets the word loop unroll to straight-line popcounts: | ncodes | NW templated | NW runtime | | --- | --- | --- | | 2, 4 (0 words) | 0.489 / 0.433 | 0.489 / 0.413 | | 10 (1 word) | **0.873** | 1.730 | | 12 (1 word) | **0.696** | 1.725 | | 20 (2 words) | **1.065** | 2.086 | | 33 (4 words) | 2.644 | 2.589 | Fig 2: Ragged table comparing just NW templating vs not having it templated So `NW` is templated for 0-3 words and left runtime from 4 up, which is where it stops paying. ## Results AMD Genoa (EPYC, Zen 4), `mode/opt`, interleaved A/B against the parent commit, one code size per process, 250 samples per cell, min ns per (query, candidate) pair. Word-multiple sizes: | ncodes | AVX2 | AVX512 | | --- | --- | --- | | 32 | **-12.6%** (1.440 -> 1.258) | **-23.3%** (1.352 -> 1.037) | | 64 | **-40.6%** (3.421 -> 2.031) | **-63.1%** (5.680 -> 2.094) | | 40 | +0.0% | **-4.0%** (2.954 -> 2.837) | | 48 | +0.1% | **-4.7%** (3.530 -> 3.365) | | 56 | +0.0% | **+4.0%** (3.944 -> 4.101) | | 8, 16, 24, 128, 160 | within +/-0.3% | within +/-0.8% | Fig 3: result table comparing against baseline (aka before this diff) ncodes 32 and 64 are the sizes whose codes are exactly one vector register wide, which is where the gather cost the most. Absolute cost of every code size with this diff, so the ragged ones can be read against the word-multiple ones: | ncodes | kind | AVX2 ns/pair | AVX2 ns/byte | AVX512 ns/pair | AVX512 ns/byte | | --- | --- | --- | --- | --- | --- | | 2 | ragged | 0.487 | 0.243 | 0.476 | 0.238 | | 4 | ragged | 0.434 | 0.108 | 0.441 | 0.110 | | 8 | word-multiple | 0.240 | 0.030 | 0.215 | 0.027 | | 10 | ragged | 0.879 | 0.088 | 0.789 | 0.079 | | 12 | ragged | 0.693 | 0.058 | 0.693 | 0.058 | | 16 | word-multiple | 0.713 | 0.045 | 0.573 | 0.036 | | 20 | ragged | 1.065 | 0.053 | 1.127 | 0.056 | | 24 | word-multiple | 2.082 | 0.087 | 2.081 | 0.087 | | 32 | word-multiple | 1.257 | 0.039 | 1.036 | 0.032 | | 33 | ragged | 2.644 | 0.080 | 3.160 | 0.096 | | 40 | word-multiple | 2.650 | 0.066 | 2.837 | 0.071 | | 48 | word-multiple | 3.110 | 0.065 | 3.361 | 0.070 | | 56 | word-multiple | 3.452 | 0.062 | 4.096 | 0.073 | | 64 | word-multiple | 2.039 | 0.032 | 2.102 | 0.033 | | 128 | word-multiple | 6.033 | 0.047 | 5.348 | 0.042 | | 160 | word-multiple | 8.164 | 0.051 | 5.731 | 0.036 | Fig 4: result table showing the ns/pair and ns/byte performance. ragged has no "before" because it is newly supported with this diff. Read down the ns/byte columns and the sizes with their own kernel (8, 16, 32, 64) sit at 0.027-0.045, the new ragged kernel at 0.053-0.096 once past a single word, and the shared runtime-word-count kernel (24, 40, 48, 56, 128, 160) at 0.036-0.087. That last group is the weakest: ncodes 24 costs 2.082 ns/pair against 1.257 for ncodes 32, and ncodes 56 costs 3.452 against 2.039 for ncodes 64, both more expensive than a larger code. It vectorizes across candidates for the same reason described above, but its word count is not known at compile time so the same fix does not apply. Left alone here. The two ragged sizes below one word (2 and 4) are dominated by fixed per-call work rather than the compare itself, which is why their ns/byte is high. Reviewed By: alibeklfc Differential Revision: D115982541 fbshipit-source-id: 081c600973fbdf30852527fb6beaae4e29cadfa3 |
||
|
Michael Norris
|
2027d3b331 |
Make cuVS dispatch opt-in rather than build-implied (#5529)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5529 `GpuIndexConfig::use_cuvs`, `GpuClonerOptions::use_cuvs` and `GpuDistanceParams::use_cuvs` each defaulted to `true` whenever the library was compiled with `USE_NVIDIA_CUVS`, and to `false` otherwise. That makes the choice of backend a property of how the binary was built rather than a choice the caller made. Code that constructs a default `GpuIndexFlatConfig()` and never mentions cuVS starts dispatching to a different implementation, with its own numerical behaviour, the moment it is linked against a cuVS-enabled build -- and there is no process-wide way to opt back out, since the flag lives per-object with no environment variable or global setter behind it. Default all three to `false`, so an unset flag means "no". The flag is otherwise unchanged: callers that want cuVS set it explicitly, which is what every cuVS test already does. Builds without cuVS are unaffected, since the flag was already `false` there. `GpuIndexCagra` never consults `use_cuvs` -- it is unconditionally cuVS-backed -- so CAGRA is unaffected. One knock-on worth flagging: `test_gpu_index_ivfsq.py` and `test_index_cpu_to_gpu.py` were picking up cuVS from the default rather than setting it, so when built against a cuVS-enabled library they now exercise the classical path. Neither asserts which backend ran, so both still pass. Threading an explicit `use_cuvs=True` through them, so those variants keep earning their name, is a follow-up. Reviewed By: juancarpio27 Differential Revision: D116567346 fbshipit-source-id: 6cc3419a5563785f81626582885bec2a1ab0770d |
||
|
Bhavik Sheth
|
920a631851 |
Don't raise HNSW entry_point to a point that isn't linked yet (#5524)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5524 `hnsw_add_vertices_deterministic` promoted `entry_point` to `order[i0]` before that point had any links. Within a single add() the promoted point gets linked moments later by the same bucket's batches, so nothing is lost. Across add() calls it orphans the whole graph built by earlier calls: the new entry point has no path back to any of it, and every subsequent search starts there. Defer the raise by one prefix-doubling batch. The first batch is exactly {order[i0]} (grow = done == 0 ? 1 : done), so once it returns the point is linked into the existing graph and is a valid entry point. Later batches in the same bucket then see the raised max_level, which preserves upper-level connectivity -- deferring to the end of the bucket instead would lose that. Bootstrap (entry_point == -1) is unchanged: with an empty graph there is nothing to orphan and nothing to link to. entry_point and max_level end up at the same values as before; only the moment of promotion moves. Indexes built with a single add() are bit-identical. Determinism is preserved. Found via OVIS Search, which builds 4.15B-vector HNSW32 shards 1M vectors per add(). Recall against exhaustive ground truth sat at 68% and would not respond to efConstruction 40/256/512 or efSearch 120..1600. Modelling it as "the first P vectors of each shard are unreachable" gives recall = (N-P)/N and fits five independent production indexes at P = 62.2M +/- 1.3M, across 2x in N and both fp32 and SQ8. Why this survived so long: it is only reachable through repeated add(). Both existing deterministic-build tests, test_graph_based.py and bench_hnsw_deterministic.py, call add() exactly once. Reviewed By: mnorris11 Differential Revision: D116192917 fbshipit-source-id: 99503f11d322a26fb17b87ebff74808f4cbd9a5c |
||
|
Michael Norris
|
a39e260688 |
Fold multi-GPU CAGRA build into train(), delete trainMultiGpu (#5521)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5521 Previous diff: D114685755. Now aarch64 passes. `GpuIndexCagra` had two multi-GPU build entry points totalling 16 positional arguments, neither of which fits the `Index` API. This removes one, folds the other into `train()`, and speeds up the build. **Usage.** Listing more than one device in the config selects the multi-GPU build; `train()` routes to it: ```python devices = faiss.Int32Vector() for i in range(8): devices.push_back(i) an = faiss.AllNeighborsCagraConfig() an.n_clusters = 16 # 0 = auto: max(2 * n_devices, 4) an.overlap_factor = 2 # must be >= 2 an.ivf_pq_search_batch_size = 8192 # 0 = cuVS default; caps IVF-PQ memory config = faiss.GpuIndexCagraConfig() config.graph_degree = 32 config.intermediate_graph_degree = 32 config.build_algo = faiss.graph_build_algo_IVF_PQ config.devices = devices # >1 device selects the multi-GPU build config.all_neighbors_params = an index = faiss.GpuIndexCagra(res, d, faiss.METRIC_L2, config) index.train(xb) # xb must stay alive until copyTo() completes cpu_index = faiss.IndexHNSWCagra() cpu_index.base_level_only = True index.copyTo(cpu_index) # required: the GPU index is not searchable on this path ``` Float32-only, does not copy `x`, and leaves `index_` empty so `copyTo()` is the only valid follow-up. Single-GPU behaviour is unchanged when `devices` is empty. **Deleted `trainMultiGpu`.** Sharded SNMG CAGRA produces per-shard graphs with zero cross-shard edges by construction, so it needs post-hoc stitching just to be usable, and it lost to the `all_neighbors` path on both build time and recall. It had no callers outside the benchmark and one test. **Folded `trainAllNeighbors` into `train()`.** Its 6 trailing bare scalars now live in the constructor-time config struct, which is the established Faiss GPU convention: | Old argument | Now | | --- | --- | | `devices` | `GpuIndexCagraConfig::devices`, also the dispatch predicate | | `build_algo` (0/1/2) | `GpuIndexCagraConfig::build_algo` | | `refinement_rate` | `AllNeighborsCagraConfig::refinement_rate` | | `n_clusters`, `overlap_factor`, `ivfpq_search_batch` | new `AllNeighborsCagraConfig` | This also kills a live footgun: the old `int build_algo` used an encoding (0=NN-descent, 1=brute-force, 2=IVF-PQ) that disagreed with the `graph_build_algo` enum in the same header (0=IVF_PQ, 1=NN_DESCENT). Both callers set `config.build_algo` and then passed an unrelated int, and the config field was silently dead. `BRUTE_FORCE` is appended to `graph_build_algo` (at the end, so existing values do not renumber) and the config field is now the single source of truth. **Graph pruning no longer copies the graph off the device.** Previously only detour counting ran on the GPU and pruning plus reverse-graph construction ran on the host, which was 35% of total build time. This part is **temporary and self-removing**. cuVS has no public API to prune a graph that is already on the device: its exported `helpers::optimize` takes host matrices, and the dispatch behind it erases the mdspan accessor to host memory, so cuVS's own device code path is unreachable from outside. Until that is fixed upstream, this reaches into cuVS's internal headers when the build has them available, and otherwise falls back to the public host API -- correct either way, just slower on the fallback. **Open-source builds get the fallback**, since cuVS does not install those headers; the `train()` docs say so. An upstream patch adding a `device_matrix_view` overload to `cuvs::neighbors::cagra::helpers::optimize` is prepared and will be submitted to rapidsai/cuvs. When it ships, the internal include, the build flag guarding it, and the fallback branch all get deleted and every build gets the fast path. At 50M vectors on 8 GPUs the optimize step goes from 119.1s to 1.65s (72x) and end-to-end build->serialize from 8.0 to 5.4 minutes, recall unchanged. **Collapsed the benchmark to one path.** With the stitching approaches gone, `bench_approaches.py` is a single-path tool for validating and tuning the production build, and reports a per-phase build breakdown plus an efSearch sweep of recall and QPS. Deliberately *not* merged into the config: - `ivf_pq_params` / `ivf_pq_search_params` are still not consulted on this path. cuVS derives them from the dataset shape and those derived values beat the static defaults in the faiss structs, so only the knobs cuVS cannot infer are overridden. - `faiss::cagra_build_algo` only has `{IVF_PQ, NN_DESCENT}`, so a `BRUTE_FORCE` config would silently degrade to NN-descent on the single-GPU path; `train_ex()` now rejects it there. `GpuIndexBinaryCagra` shares this config struct and has no multi-GPU build, so it rejects `devices.size() > 1` rather than silently building on one device. Behaviour changes: the default `build_algo` on the multi-GPU path is now `IVF_PQ` (the config default) rather than NN-descent (the old argument default), and `IndexHNSW::num_base_level_search_entrypoints` goes 32 -> 256, which measured better on both recall and QPS at every efSearch. Reviewed By: junjieqi Differential Revision: D116187873 fbshipit-source-id: 265be7dc0670de262cb1a937ca4b2f530578e237 |
||
|
Trevor Mathisen
|
1f93154314 |
Revert D114685755: Fold multi-GPU CAGRA build into train(), delete trainMultiGpu
Differential Revision: D114685755 Original commit changeset: 4eb72793284d Original Phabricator Diff: D114685755 fbshipit-source-id: e0d325ef77a323885736a7eb04deb21301d36fc4 |
||
|
Michael Norris
|
6644dcfa55 |
Fold multi-GPU CAGRA build into train(), delete trainMultiGpu (#5500)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5500 `GpuIndexCagra` had two multi-GPU build entry points totalling 16 positional arguments, neither of which fits the `Index` API. This removes one, folds the other into `train()`, and speeds up the build. **Usage.** Listing more than one device in the config selects the multi-GPU build; `train()` routes to it: ```python devices = faiss.Int32Vector() for i in range(8): devices.push_back(i) an = faiss.AllNeighborsCagraConfig() an.n_clusters = 16 # 0 = auto: max(2 * n_devices, 4) an.overlap_factor = 2 # must be >= 2 an.ivf_pq_search_batch_size = 8192 # 0 = cuVS default; caps IVF-PQ memory config = faiss.GpuIndexCagraConfig() config.graph_degree = 32 config.intermediate_graph_degree = 32 config.build_algo = faiss.graph_build_algo_IVF_PQ config.devices = devices # >1 device selects the multi-GPU build config.all_neighbors_params = an index = faiss.GpuIndexCagra(res, d, faiss.METRIC_L2, config) index.train(xb) # xb must stay alive until copyTo() completes cpu_index = faiss.IndexHNSWCagra() cpu_index.base_level_only = True index.copyTo(cpu_index) # required: the GPU index is not searchable on this path ``` Float32-only, does not copy `x`, and leaves `index_` empty so `copyTo()` is the only valid follow-up. Single-GPU behaviour is unchanged when `devices` is empty. **Deleted `trainMultiGpu`.** Sharded SNMG CAGRA produces per-shard graphs with zero cross-shard edges by construction, so it needs post-hoc stitching just to be usable, and it lost to the `all_neighbors` path on both build time and recall. It had no callers outside the benchmark and one test. **Folded `trainAllNeighbors` into `train()`.** Its 6 trailing bare scalars now live in the constructor-time config struct, which is the established Faiss GPU convention: | Old argument | Now | | --- | --- | | `devices` | `GpuIndexCagraConfig::devices`, also the dispatch predicate | | `build_algo` (0/1/2) | `GpuIndexCagraConfig::build_algo` | | `refinement_rate` | `AllNeighborsCagraConfig::refinement_rate` | | `n_clusters`, `overlap_factor`, `ivfpq_search_batch` | new `AllNeighborsCagraConfig` | This also kills a live footgun: the old `int build_algo` used an encoding (0=NN-descent, 1=brute-force, 2=IVF-PQ) that disagreed with the `graph_build_algo` enum in the same header (0=IVF_PQ, 1=NN_DESCENT). Both callers set `config.build_algo` and then passed an unrelated int, and the config field was silently dead. `BRUTE_FORCE` is appended to `graph_build_algo` (at the end, so existing values do not renumber) and the config field is now the single source of truth. **Graph pruning no longer copies the graph off the device.** Previously only detour counting ran on the GPU and pruning plus reverse-graph construction ran on the host, which was 35% of total build time. This part is **temporary and self-removing**. cuVS has no public API to prune a graph that is already on the device: its exported `helpers::optimize` takes host matrices, and the dispatch behind it erases the mdspan accessor to host memory, so cuVS's own device code path is unreachable from outside. Until that is fixed upstream, this reaches into cuVS's internal headers when the build has them available, and otherwise falls back to the public host API -- correct either way, just slower on the fallback. **Open-source builds get the fallback**, since cuVS does not install those headers; the `train()` docs say so. An upstream patch adding a `device_matrix_view` overload to `cuvs::neighbors::cagra::helpers::optimize` is prepared and will be submitted to rapidsai/cuvs. When it ships, the internal include, the build flag guarding it, and the fallback branch all get deleted and every build gets the fast path. At 50M vectors on 8 GPUs the optimize step goes from 119.1s to 1.65s (72x) and end-to-end build->serialize from 8.0 to 5.4 minutes, recall unchanged. **Collapsed the benchmark to one path.** With the stitching approaches gone, `bench_approaches.py` is a single-path tool for validating and tuning the production build, and reports a per-phase build breakdown plus an efSearch sweep of recall and QPS. Deliberately *not* merged into the config: - `ivf_pq_params` / `ivf_pq_search_params` are still not consulted on this path. cuVS derives them from the dataset shape and those derived values beat the static defaults in the faiss structs, so only the knobs cuVS cannot infer are overridden. - `faiss::cagra_build_algo` only has `{IVF_PQ, NN_DESCENT}`, so a `BRUTE_FORCE` config would silently degrade to NN-descent on the single-GPU path; `train_ex()` now rejects it there. `GpuIndexBinaryCagra` shares this config struct and has no multi-GPU build, so it rejects `devices.size() > 1` rather than silently building on one device. Behaviour changes: the default `build_algo` on the multi-GPU path is now `IVF_PQ` (the config default) rather than NN-descent (the old argument default), and `IndexHNSW::num_base_level_search_entrypoints` goes 32 -> 256, which measured better on both recall and QPS at every efSearch. Reviewed By: alibeklfc Differential Revision: D114685755 fbshipit-source-id: 4eb72793284d2e9f19c77e20fdcc6c28be4bf3bb |
||
|
Michael Norris
|
80a16564f8 |
Enable Faiss dynamic dispatch for OVIS and VeST fbpkgs (#5515)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5515 Enable `faiss.dynamic_dispatch=true` on the three `vector_search` fbpkgs that link Faiss, and add a probe that logs which SIMD kernels each process actually dispatched to. Also adds some logs in VeST. **fbpkgs covered** - `ovis_cli` (`cli/BUCK`) - `vector_search_offline_knn` (`offline_knn/BUCK`) - `vector_search_index_trainer` (`training/BUCK`) — the VeST trainer Without the flag, Faiss is compiled with static AVX2 SIMD selection. With it, all compiled-in SIMD variants (AVX2, AVX512, AVX512-SPR) are built and the best supported one is selected at runtime via CPUID. **This is not a speedup, and is not intended as one.** These workloads *build* indexes rather than search them, and the AVX-512 win for SQ4 lives on the scan path, not the encode path. Measured end-to-end as perf-neutral — see the test plan. The reasons to land it are consistency across the `vector_search` fbpkgs, unlocking AVX-512 for any future search-side work in these binaries, and the observability below. **SIMD probe.** `log_faiss_simd_level()` in `training/utils.py` logs `level`, `dispatched_level`, `has_dynamic_dispatch` and `compile_options`. It is called from `assignment_worker.py` (datasink worker) and from `datasinks/common.py::_build_merged_file_blocking` (the spawned leaf-build children are separate processes and do not inherit parent Faiss state, so each reports its own; guarded to log once per process rather than once per cluster). This probe is load-bearing: Faiss logs nothing about its own dispatch state, so the earlier VeST DD A/B could not distinguish "DD did not help" from "DD never engaged". With the probe, that ambiguity is resolved — DD demonstrably engages, and the null result is real. Reviewed By: bshethmeta Differential Revision: D107286701 fbshipit-source-id: 48cada6220c2b379f9774514edd48f2f5858f39a |
||
|
Michael Norris
|
e2f9ccaccc |
Fix SQtqmse codebook scale: fold 1/sqrt(d) into Lloyd-Max table (#5517)
Summary: Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5517 `SQtqmse` reconstructions have inflated norms and badly degraded recall since D102408184. Reported as a 1.14.2 -> 1.14.3 regression: at `d=64`, median decoded norm of a unit-norm input went `0.9965` -> `1.1503` (+15%), while `SQtqmse8` looked "essentially unchanged". Fixes https://github.com/facebookresearch/faiss/issues/5317 Root cause - D102408184 replaced the `tqmse` codebook construction with hardcoded Lloyd-Max tables: ``` - scalar_quantizer::train_TurboQuantMSE(d, 4, trained); + populate_lloyd_max_trained(4, trained); ``` The old codebook was built for one coordinate of a unit-norm vector in R^d, so its centroids scaled as `1/sqrt(d)`. The new tables are optimal for `N(0, 1)` and do not depend on `d` at all. That is correct for `_eden` and `_tq`, which rescale each vector to unit variance before lookup. Plain `tqmse` does not: it encodes raw unit-norm vectors, whose components are ~`1/sqrt(d)` (~`0.125` at `d=64`), against a codebook ~50x too wide. Nearly everything falls in the innermost cells, so the codebook collapses to a few levels and every component decodes too large: | d | bits | distinct codes used (before D102408184 -> after) | | --- | --- | --- | | 64 | 4 | 16/16 -> **4/16** | | 768 | 4 | 16/16 -> **2/16** | | 768 | 8 | 256/256 -> **32/256** | Gets worse as `d` grows. Fix - Give `populate_lloyd_max_trained` a `scale` argument that multiplies the table. The five `tqmse` cases pass `1/sqrt(d)` (the standard deviation of a unit-norm vector's components) so the codebook once again (like 1.14.2) matches the data it encodes. **No change for `_eden` / `_tq`.** They pass no `scale`, so it defaults to `1` and every table entry is bit-identical to today. Scaling `trained` rather than the encode path is what keeps this a one-line fix per callsite: every SIMD specialization and distance computer reads `this->centroids`, a pointer into `trained`. Notes - - NOT A REVERT TO 1.14.2, but that is fine. 1.14.2 trained on the exact unit-sphere marginal; this scales a Gaussian approximation of it, so centroids differ by up to ~20% at 8 bits. The approximation is not worse: max component error is lower than 1.14.2 and recall is restored (see test plan). The only consequence is that `tqmse` codes are not comparable across 1.14.2 / 1.14.3 / this version -- harmless, because the codebook ships with the index (below). - SERIALIZATION STILL COMPATIBLE: Existing 1.14.3-written `tqmse` indexes stay readable and self-consistent: `read_ScalarQuantizer` loads `trained` verbatim, so decode matches how they were encoded, and `trained` length is unchanged so size validation still passes. They are degraded, not corrupt, and need re-indexing to benefit. No version guard added. Reviewed By: alibeklfc Differential Revision: D115440646 fbshipit-source-id: a559f6a6a50a65dc3ab32ff3f17cda8796b2d4ae |