[GitHub] [lucene] jtibshirani commented on a diff in pull request #833: LUCENE-10411: Add NN vectors support to ExitableDirectoryReader

[GitBox <gi...@apache.org>]

jtibshirani commented on code in PR #833:
URL: https://github.com/apache/lucene/pull/833#discussion_r865068242


##########
lucene/CHANGES.txt:
##########
@@ -128,6 +128,9 @@ Optimizations
 * LUCENE-8836: Speed up calls to TermsEnum#lookupOrd on doc values terms enums
   and sequences of increasing ords. (Bruno Roustant, Adrien Grand)
 
+* LUCENE-10411: Add nearest neighbors vectors support to ExitableDirectoryReader. 
+  (Zach Chen, Adrien Grand, Julie Tibshirani, Tomoko Uchida)

Review Comment:
   Thanks for the generous attribution, it's totally fine if you omit me though when I just review/ chat on a PR :)



##########
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java:
##########
@@ -323,6 +325,62 @@ public int nextDoc() throws IOException {
           : sortedSetDocValues;
     }
 
+    @Override
+    public VectorValues getVectorValues(String field) throws IOException {
+      final VectorValues vectorValues = in.getVectorValues(field);
+      if (vectorValues == null) {
+        return null;
+      }
+      return (queryTimeout.isTimeoutEnabled())
+          ? new ExitableVectorValues(vectorValues)
+          : vectorValues;
+    }
+
+    @Override
+    public TopDocs searchNearestVectors(
+        String field, float[] target, int k, Bits acceptDocs, int visitedLimit) throws IOException {
+
+      // when acceptDocs is null due to no doc deleted, we will instantiate a new one that would
+      // match all docs to allow timeout checking.
+      final Bits updatedAcceptDocs =
+          acceptDocs == null ? new Bits.MatchAllBits(maxDoc()) : acceptDocs;
+
+      Bits timeoutCheckingAcceptDocs =

Review Comment:
   To check I understand, is this just a loose best-effort attempt to enforce the timeout? There's no guarantee that the number of checks to `acceptDocs` correlates to progress (like in terms of the number of visited vectors). This is true of HNSW and I imagine other implementations too.



-- 
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: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org