-
Notifications
You must be signed in to change notification settings - Fork 25.5k
Expose vector values from Int7SQVectorScorerSupplier #136416
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
Conversation
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
import static org.apache.lucene.util.quantization.ScalarQuantizedVectorSimilarity.fromVectorSimilarity; | ||
|
||
public abstract sealed class Int7SQVectorScorerSupplier implements RandomVectorScorerSupplier { | ||
public abstract sealed class Int7SQVectorScorerSupplier implements RandomVectorScorerSupplier, QuantizedByteVectorValuesAccess { |
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.
shouldn't you have this interface satisfied by regular scorer supplier as well?
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.
Do you mean to generalise it beyond QuantizedByteVectorValues? Or for the case of a non-int7 scorer?
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.
Ah, I guess this is the only "custom" one we provide right now. I was thinking that we had other RandomVectorScorerSupplier
objects.
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 also have the same question as Ben, do we really need this QuantizedByteVectorValuesAccess? As we can get access to the quantized vectors through RandomVectorScorerSupplier -> values -> getSlice
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.
@mayya-sharipova RandomVectorScorerSupplier
does not have a values
field or accessor; values
is declared directly in Int7SQVectorScorerSupplier
, which is an internal type we would like not to expose, hence QuantizedByteVectorValuesAccess
(in a fashion similar to HasIndexSlice or MemorySegmentAccess).
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.
@benwtrent I see your point; we could actually generalize this a bit beyond QuantizedByteVectorValues, e.g. cover all our implementations of RandomVectorScorerSupplier
(I count 4 of them in the ES codebase), and return ByteVectorValues
.
Maybe it makes sense; eventually, we would like to get something similar into Lucene if possible...
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.
Let's got with this small interface for now, we can expand and generalise it later, if needed.
Exposes the vector values from the Int7 scorer supplier, via a "capability" interface (same pattern as used elsewhere in Lucene, e.g. MemorySegmentAccess or HasIndexSlice)
Exposes the vector values from the Int7 scorer supplier, via a "capability" interface (same pattern as used elsewhere in Lucene, e.g. MemorySegmentAccess or HasIndexSlice)
Extracted from #136411
Exposes the vector values from the Int7 scorer supplier, via a "capability" interface (same pattern as used elsewhere in Lucene, e.g. MemorySegmentAccess or HasIndexSlice)