Skip to content
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

Add IVF changes to support Faiss byte vector #2002

Conversation

naveentatikonda
Copy link
Member

Description

Add IVF changes to support Faiss byte vector which behind the scenes uses Faiss SQ8_direct_signed scalar quantizer.

Related Issues

#1659

Check List

  • New functionality includes testing.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

@naveentatikonda naveentatikonda added Features Introduces a new unit of functionality that satisfies a requirement backport 2.x v2.17.0 labels Aug 22, 2024
@naveentatikonda naveentatikonda self-assigned this Aug 22, 2024
@naveentatikonda naveentatikonda force-pushed the add-support-faiss-byte-vector-ivf branch from 17a4e8b to 60f21fb Compare August 22, 2024 04:54
@naveentatikonda naveentatikonda force-pushed the add-support-faiss-byte-vector-ivf branch 2 times, most recently from e4fd8c6 to 3b0c8da Compare August 26, 2024 16:53
@naveentatikonda naveentatikonda force-pushed the add-support-faiss-byte-vector-ivf branch from 3b0c8da to 121912f Compare August 26, 2024 17:04
@@ -320,6 +320,96 @@ void knn_jni::faiss_wrapper::CreateBinaryIndexFromTemplate(knn_jni::JNIUtilInter
faiss::write_index_binary(&idMap, indexPathCpp.c_str());
}

void knn_jni::faiss_wrapper::CreateByteIndexFromTemplate(knn_jni::JNIUtilInterface * jniUtil, JNIEnv * env, jintArray idsJ,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we move these things to IndexService just like we have for other indices?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need to refactor these methods and move to IndexService. But, including those refactoring changes into this PR will make it big so want to do it later. Junqiu already started refactoring train Indices in #1918. Will collab with Junqiu and add my changes to that PR

jni/src/faiss_wrapper.cpp Show resolved Hide resolved
@navneet1v
Copy link
Collaborator

@naveentatikonda can we put a small benchmark run for this to showcase that ByteVector with IVF is working and we are seeing drop in memory usage.

@naveentatikonda
Copy link
Member Author

@naveentatikonda can we put a small benchmark run for this to showcase that ByteVector with IVF is working and we are seeing drop in memory usage.

This is the RSS metrics for Cohere-wiki-768 dataset (475,858 vectors) and the memory usage is as expected.
IVF memory requirement - 0.4 gb (appx.)
Heap - 1GB
Total memory provided - 1.7 GB

image

Signed-off-by: Naveen Tatikonda <[email protected]>
Copy link
Member

@jmazanec15 jmazanec15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@junqiu-lei junqiu-lei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@naveentatikonda naveentatikonda merged commit 2b303d9 into opensearch-project:main Aug 30, 2024
31 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 30, 2024
* Add HNSW changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

* Add IVF changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
(cherry picked from commit 2b303d9)
naveentatikonda added a commit that referenced this pull request Aug 30, 2024
* Add HNSW changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

* Add IVF changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
(cherry picked from commit 2b303d9)

Co-authored-by: Naveen Tatikonda <[email protected]>
akashsha1 pushed a commit to akashsha1/k-NN that referenced this pull request Sep 16, 2024
* Add HNSW changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

* Add IVF changes to support Faiss byte vector

Signed-off-by: Naveen Tatikonda <[email protected]>

* Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Features Introduces a new unit of functionality that satisfies a requirement v2.17.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants