jtibshirani commented on a change in pull request #267:
URL: https://github.com/apache/lucene/pull/267#discussion_r699760822
##########
File path: lucene/core/src/java/org/apache/lucene/util/hnsw/HnswGraph.java
##########
@@ -154,7 +210,7 @@ public static NeighborQueue search(
visited.set(friendOrd);
float score = similarityFunction.compare(query,
vectors.vectorValue(friendOrd));
- if (results.size() < numSeed || bound.check(score) == false) {
+ if (results.size() < topK || bound.check(score) == false) {
candidates.add(friendOrd, score);
if (acceptOrds == null || acceptOrds.get(friendOrd)) {
Review comment:
Do we also need to check if `level > 0` here? Maybe it's more solid to
pass in `null` for `acceptOrds` when searching upper levels, so we don't need
to remember to always check this.
##########
File path: lucene/core/src/java/org/apache/lucene/util/hnsw/HnswGraph.java
##########
@@ -206,11 +275,65 @@ public void seek(int level, int targetNode) {
upto = -1;
}
+ /**
+ * Positions the graph on the given level. Must be used before iterating
over nodes on this level
+ * with the method {@code nextNodeOnLevel()}.
+ *
+ * <p>Package private access to use only for tests
+ */
+ void seekLevel(int level) {
Review comment:
It feels a little confusing that we have both `seekLevel(level)`, which
positions the level, and `seek(level, targetNode)` which doesn't. Maybe we can
refine this API once as part of the on-disk PR, when we'll have a better idea
of the final graph layout.
--
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
