Commit Graph
5 Commits
Author SHA1 Message Date
Matthijs Douze c0575f2c13 avoid runtime checks in VisitedTable (#5234)
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5234

There are two VisitedTable implementations. Avoid runtime checks between the two by making them separate classes and templatizing the visited tables on them.

Reviewed By: mnorris11

Differential Revision: D105961848

fbshipit-source-id: 8c82c4ba2cf13e221a4f6a4e534f49e58073355d
2026-05-30 02:02:42 -07:00
Alibek Zhakubayev 255250985a Fix bugs in NSG search_on_graph and sync_prune (#5063)
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/5063

Fix two bugs in `fbcode/faiss/impl/NSG.cpp`:

**Bug 1: `init_ids[i]` → `init_ids[num_ids]` in `search_on_graph`**

The init loop in `search_on_graph` reads neighbors of the enterpoint from the
knn_graph. When an entry has `id >= ntotal`, it is skipped via `continue`. The
loop variable `i` advances but `num_ids` (the write pointer) does not. The old
code wrote `init_ids[i] = id`, placing valid entries at non-contiguous positions
and leaving gaps in between. The gap-filling loop that follows starts from
`num_ids`, so it never overwrites the internal gaps.

Example with neighbors `[5, 99999, 3, 99999, 7]` and `ntotal=100`:

| i | id    | old: `init_ids[i]=id` | new: `init_ids[num_ids]=id` |
|---|-------|-----------------------|-----------------------------|
| 0 | 5     | `init_ids[0] = 5`     | `init_ids[0] = 5`           |
| 1 | 99999 | skip (gap at [1])     | skip                        |
| 2 | 3     | `init_ids[2] = 3`     | `init_ids[1] = 3`           |
| 3 | 99999 | skip (gap at [3])     | skip                        |
| 4 | 7     | `init_ids[4] = 7`     | `init_ids[2] = 7`           |

Old result: `[5, 0, 3, 0, 7, ...]` — gaps contain 0 (from `vector<int>`
zero-initialization). The consumption loop reads these zeros as node IDs,
biasing the search pool toward node 0 during graph construction and degrading
graph quality.

Fixed result: `[5, 3, 7, ...]` — valid entries packed contiguously, gap-filling
starts from the correct position.

**Bug 2: `sync_prune` out-of-bounds access**

`sync_prune` accesses `pool[start]` without bounds checking. Two cases cause
out-of-bounds reads:
- Pool is empty after augmentation
- Pool contains only the query node itself (e.g., `ntotal=1`)

In both cases `start` advances past `pool.size()` and `pool[start]` is an
out-of-bounds vector read — undefined behavior that crashes under ASAN and
silently corrupts the graph in release builds.

Trace for `ntotal=1, L=1`:
1. `search_on_graph` returns `pool = [{id:0, dist:0}]`
2. `sync_prune(q=0)`: `pool[0].id == q` → `start++` → `start=1 == pool.size()`
3. Old code: `result.push_back(pool[1])` — OOB read
4. Fix: guard checks `start >= pool.size()`, fills graph row with `EMPTY_ID`

**Other fixes:**
- Replace `min = 1e6` (float-to-int truncation) with `std::numeric_limits<int>::max()`
- Remove `srand(0x1998)` from the NSG constructor (global RNG side effect)

Reviewed By: mnorris11

Differential Revision: D100024850

fbshipit-source-id: 0d290801658e381e198b6c6ab54ebe981e0f09f3
2026-04-09 10:00:33 -07:00
Max BuckleyandClaude Opus 4.6 b3a2914982 fix: resolve compiler warnings in test files (#4944)
Summary:
- Fix compiler warnings in all 20 FAISS C++ test files
- Fixes include `-Wshadow`, `-Wunused-parameter`, and signed/unsigned comparisons

All changes are mechanical. No functional changes.

Part 13/13 of the compiler warnings cleanup (split from https://github.com/facebookresearch/faiss/issues/4810 per maintainer request).

Pull Request resolved: https://github.com/facebookresearch/faiss/pull/4944

Test Plan:
- [x] No functional changes — all fixes are mechanical renames and annotations

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Reviewed By: mnorris11

Differential Revision: D96996609

Pulled By: alibeklfc

fbshipit-source-id: 72555246a4ccf540eb5422e64018c8d5e2660dce
2026-03-20 10:21:55 -07:00
Michael Norris eff0898a13 Enable linting: lint config changes plus arc lint command (#3966)
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/3966

This actually enables the linting.

Manual changes:
- tools/arcanist/lint/fbsource-licenselint-config.toml
- tools/arcanist/lint/fbsource-lint-engine.toml

Automated changes:
`arc lint --apply-patches --take LICENSELINT --paths-cmd 'hg files faiss'`

Reviewed By: asadoughi

Differential Revision: D64484165

fbshipit-source-id: 4f2f6e953c94ef6ebfea8a5ae035ccfbea65ed04
2024-10-22 09:46:48 -07:00
Matthijs Douze adcde10e50 Allow to replace graph structure for NSG graphs (#3975)
Summary:
Pull Request resolved: https://github.com/facebookresearch/faiss/pull/3975

Graph-based indices are often quite bulky in terms of storage becausee the out-degree of the edges is high (32 or more) and edges are just encoded as 32-bit ints.

This diff makes it possible to replace the graph structure of NSG with a compressed version, but only for search (the full graph needs to be present at addition time).

It is easier to do it for NSG than for HNSW for several reasons:
- NSG's graph is not hierarichal -- HNSW is and the edges for the different levels are interleaved
- the NSG graph object is already isolated well (thanks KingLittleQ!)
- NSG cannot be built incrementally so it is easier to convert the graph after all adds are done in one go.

The custom compressed graph is currently only implemented as a test, but could be integrated in the main Faiss as an option to NSG.

Reviewed By: algoriddle

Differential Revision: D64646137

fbshipit-source-id: c10c2a485b44561d32941ce1e7a0e3fe512cf0ac
2024-10-21 06:33:11 -07:00