Skip to content

Conversation

mnorris11
Copy link

@mnorris11 mnorris11 commented Feb 6, 2025

GitHub Author: Kaival Parikh kaivalp2000@gmail.com

(This required some updates to an internal unit test to pass the internal Meta CI, so I had to unlink and re-export as this new PR)

Summary from #4167 below:

Summary:
Add ability to search HNSW indexes using a plain SearchParameters object (i.e. only an IDSelector)

Issue: Currently if a plain SearchParameters is used to query an HNSW index, an error is thrown -- when the user's intent was only to filter some documents, and rely on index settings for remaining parameters (like efSearch, check_relative_distance, search_bounded_queue)

Motivation: Faiss provides an amazing index factory and parameter setter to abstract away internals of the index type and settings used, like:

Index* index = index_factory(256, "HNSW32");
ParameterSpace().set_index_parameters(index, "efConstruction=200,efSearch=150");

Now if a user wants to perform a filtered search on this opaque index using:

SearchParameters parameters;
parameters.sel = new IDSelectorRange(10, 20);
index->search(nq, xq, k, d, id, &parameters);

they are met with an error:

faiss/IndexHNSW.cpp:251: Error: '!(params)' failed: params type invalid

An easy way to reproduce this issue is to replace Flat -> HNSW here and run example_c like:

make -C build example_c
./build/c_api/example_c

This PR allows passing a plain SearchParameters to HNSW indexes, and use index settings as a fallback

Reviewed By: asadoughi

Differential Revision: D69266072

Pulled By: mnorris11

Summary:
Add ability to search HNSW indexes using a plain [`SearchParameters`](https://github.com/facebookresearch/faiss/blob/6c046992a71e672df504a0101cddf6f2f2e90601/faiss/Index.h#L64-L69) object (i.e. only an [`IDSelector`](https://github.com/facebookresearch/faiss/blob/6c046992a71e672df504a0101cddf6f2f2e90601/faiss/Index.h#L66))

Issue: Currently if a plain `SearchParameters` is used to query an HNSW index, [an error is thrown](https://github.com/facebookresearch/faiss/blob/6c046992a71e672df504a0101cddf6f2f2e90601/faiss/IndexHNSW.cpp#L251) -- when the user's intent was only to filter some documents, and rely on index settings for remaining parameters (like `efSearch`, `check_relative_distance`, `search_bounded_queue`)

Motivation: Faiss provides an amazing [index factory](https://github.com/facebookresearch/faiss/wiki/The-index-factory) and [parameter setter](https://github.com/facebookresearch/faiss/wiki/Index-IO,-cloning-and-hyper-parameter-tuning) to abstract away internals of the index type and settings used, like:
```cpp
Index* index = index_factory(256, "HNSW32");
ParameterSpace().set_index_parameters(index, "efConstruction=200,efSearch=150");
```

Now if a user wants to perform a filtered search on this _opaque_ index using:
```cpp
SearchParameters parameters;
parameters.sel = new IDSelectorRange(10, 20);
index->search(nq, xq, k, d, id, &parameters);
```

they are met with an error:
```
faiss/IndexHNSW.cpp:251: Error: '!(params)' failed: params type invalid
```

An easy way to reproduce this issue is to replace `Flat` -> `HNSW` [here](https://github.com/facebookresearch/faiss/blob/6c046992a71e672df504a0101cddf6f2f2e90601/c_api/example_c.c#L60) and run `example_c` like:
```
make -C build example_c
./build/c_api/example_c
```

This PR allows passing a plain `SearchParameters` to HNSW indexes, and use index settings as a fallback


Reviewed By: asadoughi

Differential Revision: D69266072

Pulled By: mnorris11
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69266072

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D69266072

mnorris11 pushed a commit to mnorris11/faiss that referenced this pull request Feb 7, 2025
Summary: Pull Request resolved: facebookresearch#4174

Reviewed By: asadoughi

Differential Revision: D69266072

Pulled By: mnorris11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants