-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Pull out common subclasses for our custom flat & hnsw vector formats #132663
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
base: main
Are you sure you want to change the base?
Conversation
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
.../src/main/java/org/elasticsearch/index/codec/vectors/es816/ES816BinaryFlatVectorsScorer.java
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
protected abstract KnnVectorsFormat flatVectorsFormat(); |
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.
I wonder if we just pass KnnVectorsFormat flatVectorFormat
as a ctor parameter? The inheritor could still just use a statically constructed object, but pass the reference for toString.
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.
I did consider that, but there's already a lot of constructors and parameters, and I didn't want to add another. Either way would work though.
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.
I did consider that, but there's already a lot of constructors and parameters, and I didn't want to add another. Either way would work though.
I am fine with this or a ctor parameter, as long as it doesn't force package private things to become visible outside of their packages.
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.
It's just exposing public classes in lucene, so it doesn't
Pull some common code into common subclasses. Whilst these are copied from Lucene, they share some duplicated functionality. We are likely to continue to have custom flat & HNSW formats for a good time yet, so we should aim to centralise our ES-specific settings and defaults into a single class.