jtibshirani commented on a change in pull request #656:
URL: https://github.com/apache/lucene/pull/656#discussion_r802075310
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/lucene91/Lucene91HnswVectorsReader.java
##########
@@ -227,16 +231,36 @@ public TopDocs search(String field, float[] target, int
k, Bits acceptDocs) thro
// bound k by total number of vectors to prevent oversizing data structures
k = Math.min(k, fieldEntry.size());
-
OffHeapVectorValues vectorValues = getOffHeapVectorValues(fieldEntry);
+
+ DocIdSetIterator acceptIterator = null;
+ int visitedLimit = Integer.MAX_VALUE;
+
+ if (acceptDocs instanceof BitSet acceptBitSet) {
Review comment:
These are all great points. The reasons to prefer option 2 make sense to
me (although I'm also not clear on the best strategy for supporting
ExitableDirectoryReader). I had a similar intuition to @msokolov that
`visitedLimit` feels like a cost-tradeoff parameter similar to efSearch/
fanout... but I don't yet see how to bridge the gap between these two concepts.
In any case, I feel pretty good about adding a parameter `visitedLimit` for
now. The concept indeed seems general, we have room to further generalize it
later if needed (maybe an approximate `costLimit`?) or revise it.
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/lucene91/Lucene91HnswVectorsReader.java
##########
@@ -227,16 +231,36 @@ public TopDocs search(String field, float[] target, int
k, Bits acceptDocs) thro
// bound k by total number of vectors to prevent oversizing data structures
k = Math.min(k, fieldEntry.size());
-
OffHeapVectorValues vectorValues = getOffHeapVectorValues(fieldEntry);
+
+ DocIdSetIterator acceptIterator = null;
+ int visitedLimit = Integer.MAX_VALUE;
+
+ if (acceptDocs instanceof BitSet acceptBitSet) {
Review comment:
These are all great points. The reasons to prefer option 2 make sense to
me (although I'm also not clear on the best strategy for supporting
ExitableDirectoryReader). I had a similar intuition to @msokolov that
`visitedLimit` feels like a cost-tradeoff parameter similar to efSearch/
fanout... but I don't yet see how to bridge the gap between these two concepts.
In any case, I feel pretty good about adding a parameter `visitedLimit` for
now. The concept indeed seems general, and we have room to further generalize
it later if needed (maybe an approximate `costLimit`?) or revise it.
--
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-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
