[GitHub] [lucene] jtibshirani commented on a diff in pull request #872: LUCENE-10527 Use 2*maxConn for last layer in HNSW

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

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


##########
lucene/core/src/java/org/apache/lucene/codecs/lucene91/Lucene91HnswVectorsWriter.java:
##########
@@ -53,12 +53,14 @@ public final class Lucene91HnswVectorsWriter extends KnnVectorsWriter {
   private final int maxDoc;
 
   private final int maxConn;
+  private final int maxConn0;
   private final int beamWidth;
   private boolean finished;
 
-  Lucene91HnswVectorsWriter(SegmentWriteState state, int maxConn, int beamWidth)
+  Lucene91HnswVectorsWriter(SegmentWriteState state, int maxConn, int maxConn0, int beamWidth)

Review Comment:
   I was thinking we could just keep a single configuration parameter here, and internally calculate `maxConn0 = 2 * M`. If we allow it to be passed as a parameter, it seems like it's important to be able to configure it, but that's not the case (it is not something users will change and should always be set to `2 * M`). Like that we could also avoid writing a new value `maxConn0` into the format, which doesn't seem necessary?
   
   If we are worried about naming, we could rename `maxConn` to `M`. From my perspective, it's okay to use single-letter variable names (with a clear comment!) when it directly corresponds to a paper's algorithm.



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