jpountz commented on code in PR #14338:
URL: https://github.com/apache/lucene/pull/14338#discussion_r1989627096


##########
lucene/core/src/java/org/apache/lucene/util/bkd/BKDConfig.java:
##########
@@ -68,6 +82,16 @@ public record BKDConfig(int numDims, int numIndexDims, int 
bytesPerDim, int maxP
     }
   }
 
+  static BKDConfig create(int numDims, int numIndexDims, int bytesPerDim, int 
maxPointsInLeafNode) {

Review Comment:
   nit: `create` suggests that this returns a new instance, what about 
`of(...)` instead?



##########
lucene/core/src/java/org/apache/lucene/util/bkd/BKDConfig.java:
##########
@@ -38,6 +39,19 @@ public record BKDConfig(int numDims, int numIndexDims, int 
bytesPerDim, int maxP
   /** Maximum number of index dimensions */
   public static final int MAX_INDEX_DIMS = 8;
 
+  static final List<BKDConfig> DEFAULT_CONFIGS =
+      List.of(
+          // config for int / float
+          new BKDConfig(1, 1, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for long / double
+          new BKDConfig(1, 1, 8, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for inetAddress
+          new BKDConfig(1, 1, 16, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for 2D points
+          new BKDConfig(2, 2, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for shapes
+          new BKDConfig(7, 4, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE));

Review Comment:
   It feels a bit arbitrary to not include HaltFloatPoint and 
LongRange/DoubleRange. I wonder if we should do it in a way that it a bit more 
agnostic to what specific implementations exist and add an entry for numbers of 
bytes per value 1, 2, 4, 8 and for dimensions 1 and 2 (plus the config for 
shapes) instead.



##########
lucene/core/src/java/org/apache/lucene/util/bkd/BKDConfig.java:
##########
@@ -38,6 +39,19 @@ public record BKDConfig(int numDims, int numIndexDims, int 
bytesPerDim, int maxP
   /** Maximum number of index dimensions */
   public static final int MAX_INDEX_DIMS = 8;
 
+  static final List<BKDConfig> DEFAULT_CONFIGS =
+      List.of(
+          // config for int / float
+          new BKDConfig(1, 1, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for long / double
+          new BKDConfig(1, 1, 8, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for inetAddress
+          new BKDConfig(1, 1, 16, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for 2D points
+          new BKDConfig(2, 2, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE),
+          // config for shapes
+          new BKDConfig(7, 4, 4, DEFAULT_MAX_POINTS_IN_LEAF_NODE));
+
   public BKDConfig {

Review Comment:
   should we aim at making this private?



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