msokolov commented on code in PR #15549:
URL: https://github.com/apache/lucene/pull/15549#discussion_r3551552180


##########
lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene99/Lucene99ScalarQuantizedVectorsReader.java:
##########
@@ -227,6 +228,11 @@ public ByteVectorValues getByteVectorValues(String field) 
throws IOException {
     return rawVectorsReader.getByteVectorValues(field);
   }
 
+  @Override
+  public Float16VectorValues getFloat16VectorValues(String field) throws 
IOException {
+    return rawVectorsReader.getFloat16VectorValues(field);

Review Comment:
   do we want to support this in the backwards codec?



##########
lucene/benchmark-jmh/src/java/org/apache/lucene/benchmark/jmh/VectorScorerBenchmark.java:
##########
@@ -221,5 +221,11 @@ public RandomVectorScorer getRandomVectorScorer(
         VectorSimilarityFunction similarityFunction, KnnVectorValues 
vectorValues, byte[] target) {
       throw new UnsupportedOperationException();
     }
+
+    @Override
+    public RandomVectorScorer getRandomVectorScorer(

Review Comment:
   should we eventually support this? Maybe add a TODO if you intend to add



##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorScorer.java:
##########
@@ -107,6 +107,50 @@ public RandomVectorScorer getRandomVectorScorer(
     return nonQuantizedDelegate.getRandomVectorScorer(similarityFunction, 
vectorValues, target);
   }
 
+  @Override
+  public RandomVectorScorer getRandomVectorScorer(
+      VectorSimilarityFunction similarityFunction, KnnVectorValues 
vectorValues, short[] target)
+      throws IOException {
+    if (vectorValues instanceof QuantizedByteVectorValues qv) {

Review Comment:
   Is this supporting fp16 queries over 1,2,4,8-bit quantized documents? Do we 
even need to support that?



##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsReader.java:
##########
@@ -215,6 +216,30 @@ public RandomVectorScorer getRandomVectorScorer(String 
field, float[] target) th
         target);
   }
 
+  @Override
+  public RandomVectorScorer getRandomVectorScorer(String field, short[] 
target) throws IOException {
+    FieldEntry fi = fields.get(field);
+    if (fi == null) {
+      return null;
+    }
+    return vectorScorer.getRandomVectorScorer(

Review Comment:
   I'm confused about how our scorers work now, so please bear with me here. If 
we have indexed fp16 vectors and we a re searching with fp16 (short[]) queries, 
why are we using `OffHeapScalarQuantizedVectorValues` ? Where is the 
quantization coming into this?



##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsReader.java:
##########
@@ -280,12 +305,67 @@ public ByteVectorValues getByteVectorValues(String field) 
throws IOException {
     return rawVectorsReader.getByteVectorValues(field);
   }
 
+  @Override
+  public Float16VectorValues getFloat16VectorValues(String field) throws 
IOException {
+    FieldEntry fi = fields.get(field);
+    if (fi == null) {
+      return null;
+    }
+    if (fi.vectorEncoding != VectorEncoding.FLOAT16) {
+      throw new IllegalArgumentException(
+          "field=\""
+              + field
+              + "\" is encoded as: "
+              + fi.vectorEncoding
+              + " expected: "
+              + VectorEncoding.FLOAT16);
+    }
+
+    Float16VectorValues rawFloatVectorValues = 
rawVectorsReader.getFloat16VectorValues(field);
+
+    if (rawFloatVectorValues.size() == 0) {

Review Comment:
   Similar question to above -- why are we quantizing here?



##########
lucene/core/src/java/org/apache/lucene/codecs/lucene104/Lucene104ScalarQuantizedVectorsReader.java:
##########
@@ -280,12 +305,67 @@ public ByteVectorValues getByteVectorValues(String field) 
throws IOException {
     return rawVectorsReader.getByteVectorValues(field);
   }
 
+  @Override
+  public Float16VectorValues getFloat16VectorValues(String field) throws 
IOException {
+    FieldEntry fi = fields.get(field);
+    if (fi == null) {
+      return null;
+    }
+    if (fi.vectorEncoding != VectorEncoding.FLOAT16) {
+      throw new IllegalArgumentException(
+          "field=\""
+              + field
+              + "\" is encoded as: "
+              + fi.vectorEncoding
+              + " expected: "
+              + VectorEncoding.FLOAT16);
+    }
+
+    Float16VectorValues rawFloatVectorValues = 
rawVectorsReader.getFloat16VectorValues(field);
+
+    if (rawFloatVectorValues.size() == 0) {

Review Comment:
   OK I think I get it -- in addition to fp16/fp16 query/doc we are also 
supporting various conversions/quantizations here, which is cool, but I wonder 
if we could separate out the introduction of the raw fp16 encoding from teh 
quantization support, just in order to make the code changes easier to manage? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to