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

[Enhancement] Enable support for Faiss, NMSLIB in neural query integ test cases #1101

Open
vibrantvarun opened this issue Jan 14, 2025 · 7 comments

Comments

@vibrantvarun
Copy link
Member

Currently, all the integ test create knn index with lucene engine in the test suite. We need to enable the creation of the index with other supported engines as well like FAISS, K-NN to ensure better testing.

@vibrantvarun vibrantvarun changed the title [Enhancement] Enable support for Faiss, NMSLIB in neural query test cases. [Enhancement] Enable support for Faiss, NMSLIB in neural query integ test cases Jan 14, 2025
@heemin32
Copy link
Collaborator

I'm not certain about that. The neural feature should be independent of the k-NN engine.

@vibrantvarun
Copy link
Member Author

@heemin32 I think neural query existence is about a wrapper around k-nn query. It would be great if test it thoroughly with other engines. When we say independent of the k-NN engine, what exactly do you mean? Is it like we should write test cases that does not have an engine at all?

@heemin32
Copy link
Collaborator

Could there be a situation where a neural query works with one engine but not another? Since, as you mentioned, it is essentially a wrapper around the k-NN query with no differing logic between engines in the neural plugin, I believe that as long as the k-NN plugin's tests cover all supported engines, no additional testing should be necessary for the neural plugin.

@vibrantvarun
Copy link
Member Author

As we know, FAISS is the most popular option nowadays. Therefore, I would suggest making faiss implementation to work with test suite

@martin-gaievski
Copy link
Member

I'm not sure if that's needed in neural. We are consuming knn for the purpose of performing vector search, we should not test knn features in neural, especially as this one will not be a low hanging fruit. Only advantage I can think of - we may know if something in native knn engine(s) is broken a bit earlier, but even that is not guaranteed, and knn team keep their codebase in pretty good shape. To me it does not justify the efforts

@vibrantvarun
Copy link
Member Author

@martin-gaievski but we have test cases with knn index with lucene engine. Then we should at all remove the dependency of testing knn query?

@vibrantvarun
Copy link
Member Author

I think we should have atleast FAISS support as there are new developments and features coming with that engine.

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

No branches or pull requests

3 participants