[GitHub] [lucene-solr] msokolov commented on a change in pull request #2282: LUCENE-9615: Expose HnswGraphBuilder index-time hyperparameters as FieldType attributes

[GitBox]

msokolov commented on a change in pull request #2282:
URL: https://github.com/apache/lucene-solr/pull/2282#discussion_r568075607



##########
File path: 
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90VectorWriter.java
##########
@@ -188,9 +190,29 @@ private void writeGraph(
       RandomAccessVectorValuesProducer vectorValues,
       long graphDataOffset,
       long[] offsets,
-      int count)
+      int count,
+      String maxConnStr,
+      String beamWidthStr)
       throws IOException {
-    HnswGraphBuilder hnswGraphBuilder = new HnswGraphBuilder(vectorValues);
+    int maxConn, beamWidth;
+    if (maxConnStr == null) {
+      maxConn = HnswGraphBuilder.DEFAULT_MAX_CONN;
+    } else if (!maxConnStr.matches("[0-9]+")) {

Review comment:
       I don't think we need this - we can allow parseInt to throw an 
exception. Let's catch `NumberFormatException` and rethrow with more context 
(which attribute caused the exception). Also HnswGraphBuilder tests for `<= 0`, 
so we don't need to check that here.

##########
File path: lucene/core/src/java/org/apache/lucene/document/VectorField.java
##########
@@ -53,6 +54,44 @@ private static FieldType getType(float[] v, 
VectorValues.SearchStrategy searchSt
     return type;
   }
 
+  /**
+   * Public method to create HNSW field type with the given max-connections 
and beam-width
+   * parameters that would be used by HnswGraphBuilder while constructing HNSW 
graph.
+   *
+   * @param dimension dimension of vectors
+   * @param searchStrategy a function defining vector proximity.
+   * @param maxConn max-connections at each HNSW graph node
+   * @param beamWidth size of list to be used while constructing HNSW graph
+   * @throws IllegalArgumentException if any parameter is null, or has 
dimension &gt; 1024.
+   */
+  public static FieldType createHnswType(
+      int dimension, VectorValues.SearchStrategy searchStrategy, int maxConn, 
int beamWidth) {
+    if (dimension == 0) {
+      throw new IllegalArgumentException("cannot index an empty vector");
+    }
+    if (dimension > VectorValues.MAX_DIMENSIONS) {
+      throw new IllegalArgumentException(
+          "cannot index vectors with dimension greater than " + 
VectorValues.MAX_DIMENSIONS);
+    }
+    if (searchStrategy == null) {
+      throw new IllegalArgumentException("search strategy must not be null");

Review comment:
       Let's also assert `searchStrategy.isHnsw()` to catch attempts to use 
`NONE` or some other unsupported future strategy.

##########
File path: lucene/core/src/test/org/apache/lucene/util/hnsw/KnnGraphTester.java
##########
@@ -132,13 +135,13 @@ private void run(String... args) throws Exception {
           if (iarg == args.length - 1) {
             throw new IllegalArgumentException("-beamWidthIndex requires a 
following number");
           }
-          HnswGraphBuilder.DEFAULT_BEAM_WIDTH = Integer.parseInt(args[++iarg]);

Review comment:
       With this change, we no longer have any need to make these static 
variables writable - let's change them to `final` in `HnswGraphBuilder`




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

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