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 TwoPhaseKnnVectorQuery #29

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Add TwoPhaseKnnVectorQuery #29

wants to merge 15 commits into from

Conversation

dungba88
Copy link
Owner

Description

Draft for two-phase vector search (see apache#13564). Created a new Query for sake of simplicity.

*
* @lucene.experimental
*/
public class TwoPhaseKnnVectorQuery extends KnnFloatVectorQuery {

Choose a reason for hiding this comment

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

I think we would need a brand new query that doesn't build on top of AbstractKnnVectorQuery. Instead, gets passed the desired knn query as a parameter & the desired target then the outer reranking query can call the knn query via rewrite.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh that's a cleaner idea actually. Let me try it in next rev.

Comment on lines 71 to 78
for (int i = 0; i < NUM_VECTORS; i++) {
float[] vector = randomFloatVector(VECTOR_DIMENSION, random);
Document doc = new Document();
doc.add(new IntField("id", i, Field.Store.YES));
doc.add(new KnnFloatVectorField(FIELD, vector, VECTOR_SIMILARITY_FUNCTION));
writer.addDocument(doc);
vectors.put(i, vector);
}
Copy link

Choose a reason for hiding this comment

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

Maybe we should flush the writers a few time to create more than one segments, this make sure the code path that results are merged from multiple segments are executed.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good points, I added the flush() and uncovered/fixed a bug with doc ord.

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

Successfully merging this pull request may close these issues.

3 participants