jeho-rpls commented on code in PR #16383:
URL: https://github.com/apache/lucene/pull/16383#discussion_r3594977885


##########
lucene/core/src/test/org/apache/lucene/search/TestVectorScorer.java:
##########
@@ -48,6 +50,10 @@ public void testFindAll() throws IOException {
         case BYTE:
           vectorScorer = 
context.reader().getByteVectorValues("field").scorer(new byte[] {1, 2});
           break;
+        case FLOAT16:
+          Float16VectorValues val = 
context.reader().getFloat16VectorValues("field");
+          vectorScorer = val.scorer(new short[] {1, 2});
+          break;

Review Comment:
   If I am reading this right, these `shorts` are interpreted as `fp16` bit 
patterns, so `{1, 2}` would decode to a nearly zero vector rather than `{1.0, 
2.0}` like the `byte` and `float` cases. 
   
   I think the test still passes because it only counts the matched docs and 
never checks scores. Would it make sense to use like `Float.floatToFloat16(1f)` 
and `Float.floatToFloat16(2f)` here instead?



##########
lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene102/Lucene102BinaryFlatVectorsScorer.java:
##########
@@ -94,6 +94,13 @@ public RandomVectorScorer getRandomVectorScorer(
     return nonQuantizedDelegate.getRandomVectorScorer(similarityFunction, 
vectorValues, target);
   }
 
+  @Override
+  public RandomVectorScorer getRandomVectorScorer(
+      VectorSimilarityFunction similarityFunction, KnnVectorValues 
vectorValues, short[] target)
+      throws IOException {
+    return null;

Review Comment:
   Minor one, but returning null here could surface as an NPE at some caller 
far from the cause. 
   
   I noticed `Lucene102BinaryQuantizedVectorsReader` in this package throws 
`UnsupportedOperationException` for the same situation. Would it be more
   consistent to throw here as well?



-- 
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