-
Notifications
You must be signed in to change notification settings - Fork 144
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
[LuceneOnFaiss - Part3] Added FaissHNSW graph. #2594
[LuceneOnFaiss - Part3] Added FaissHNSW graph. #2594
Conversation
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSW.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissSection.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSWFlatIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will look at the unit tests soon.
Looks good otherwise, minor comments
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSW.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissSection.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSW.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSWFlatIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/FaissHNSWFlatIndex.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/knn/memoryoptsearch/faiss/LuceneFaissHnswGraph.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Dooyong Kim <kdooyong@amazon.com>
2e7e464
to
517e2c4
Compare
@jmazanec15 Conclusion : With the encoding, required memory would be shrink to 475KB from 7.45GB for 1B vectors. Current Approach With DirectMonotonicWriter Will make sure to add this encoding in the next PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, need a clarification and its good to go
// Note that maxLevel=3 indicates that a vector exists level-0 (bottom), level-1 and level-2. | ||
if (maxLevel > level) { | ||
++numVectorsAtLevel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just want to make sure that the vectors are not repeated on each level so we don't count extra
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there will be no duplicate counts as the index of levels (e.g. i
) represents an internal vector id, and we increase the index sequentially.
a6beb8c
into
opensearch-project:lucene-on-faiss
Description
In this PR, I've added FaissHNSW graph and a bridge that complies to Lucene's HNSW graph interface.
With the bridge, now we can use Lucene's HNSW graph searcher to perform vector search on FAISS index.
Related Issues
RFC : #2401
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.