-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update IVF_PQ to set memory_budget in constructor, support preload feature_vectors and metadata only modes #518
Merged
jparismorgan
merged 20 commits into
main
from
jparismorgan/ivf-pq-memory-budget-to-ctor
Sep 16, 2024
Merged
Update IVF_PQ to set memory_budget in constructor, support preload feature_vectors and metadata only modes #518
jparismorgan
merged 20 commits into
main
from
jparismorgan/ivf-pq-memory-budget-to-ctor
Sep 16, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ature_vectors and metadata only modes
…arch into jparismorgan/ivf-pq-memory-budget-to-ctor
Merged
…arch into jparismorgan/ivf-pq-memory-budget-to-ctor
…arch into jparismorgan/ivf-pq-memory-budget-to-ctor
NikolaosPapailiou
approved these changes
Sep 16, 2024
I'm going to force a merge here because the failures are unrelated to this PR and I'd like this PR in the upcoming release:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Makes three changes to IVF_PQ:
memory_budget
in constructor instead of during query. This matches what Python does and improves things because now the first infinite RAM query is not slower the rest.open_for_remote_query_execution
in Python we won't load data in C++, just metadata.Also updates
ivf_pq_index_test
because it is flaky (here is a PR without these changes failing: https://github.com/TileDB-Inc/TileDB-Vector-Search/actions/runs/10809242895/job/29983933038?pr=520).Testing
Benchmarks
Before this, the first IVF_PQ query with
query_infinite_ram()
when the index URI is a remote AWS URI would be slower than all the rest (because we first had to load the data withread_index_infinite()
):After this change, we instead get this (notice that with k_factor=1 each query is the same speed, but for others it is not because we are fetching the feature_vectors for re-ranking on each query):
And if we use
index = IVFPQIndex(index_uri, config, preload_k_factor_vectors=True)
, then we this (notice that we have pre-fetched the feature_vectors so there is no query time difference):The code to run this is below: