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

Reply via email to