[GitHub] [lucene] jtibshirani commented on a change in pull request #267: LUCENE-10054 Handle hierarchy in graph construction and search

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

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