stefanvodita commented on code in PR #14238:
URL: https://github.com/apache/lucene/pull/14238#discussion_r1955848183


##########
lucene/facet/src/test/org/apache/lucene/facet/range/TestDynamicRangeUtil.java:
##########
@@ -76,13 +77,248 @@ public void 
testComputeDynamicNumericRangesWithOneLargeWeight() {
     long[] values = new long[] {45, 32, 52, 14, 455, 342, 53};
     long[] weights = new long[] {143, 23, 1, 52343, 53, 12, 2534};
 
-    // value 14 has its own bin since the weight is large, and the rest of 
values fall the other bin
-    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343, 
14L, 14L, 14D));
+    // value 14 has its own bin since the weight is large, and the rest of the 
values fall in the
+    // other bin
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343L, 
14L, 14L, 14D));
     expectedRangeInfoList.add(
-        new DynamicRangeUtil.DynamicRangeInfo(6, 2766, 32L, 455L, 
163.16666666666666D));
+        new DynamicRangeUtil.DynamicRangeInfo(6, 2766L, 32L, 455L, 
163.16666666666666D));
     assertDynamicNumericRangeResults(values, weights, 4, 55109, 
expectedRangeInfoList);
   }
 
+  public void testComputeDynamicNumericRangesWithLargeTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // More requested ranges (TopN) than values should return ranges with 
weights larger than the
+    // average weight - excluding the last range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 560L, 
277L, 277L, 277D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 508L, 
439L, 439L, 439D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 42, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSingleTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // A single requested range (TopN) should return a single range regardless 
of the weights
+    // provided
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(4, 1863L, 
277L, 794L, 499.25D));
+    assertDynamicNumericRangeResults(values, weights, 1, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithTwoTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // Two requested ranges (TopN) should return two ranges where the first 
range's weight is equal
+    // or larger than half of the total weight
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 1068L, 
277L, 439L, 358.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 2, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameWeights() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[100];
+    long[] weights = new long[100];
+    for (int i = 0; i < 100; i++) {
+      values[i] = i;
+      weights[i] = 50;
+    }
+
+    // Supplying only equal weights should return ranges with equal counts - 
excluding the last
+    // range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
0L, 24L, 12.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
25L, 49L, 37.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
50L, 74L, 62.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
75L, 99L, 87.0D));
+    assertDynamicNumericRangeResults(values, weights, 4, 5000L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameWeightsShuffled() {

Review Comment:
   This test and the next one are ensuring that we define a strict order among 
entries, considering both values and weights. Could we say that explicitly in a 
comment?



##########
lucene/facet/src/test/org/apache/lucene/facet/range/TestDynamicRangeUtil.java:
##########
@@ -76,13 +77,248 @@ public void 
testComputeDynamicNumericRangesWithOneLargeWeight() {
     long[] values = new long[] {45, 32, 52, 14, 455, 342, 53};
     long[] weights = new long[] {143, 23, 1, 52343, 53, 12, 2534};
 
-    // value 14 has its own bin since the weight is large, and the rest of 
values fall the other bin
-    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343, 
14L, 14L, 14D));
+    // value 14 has its own bin since the weight is large, and the rest of the 
values fall in the
+    // other bin
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343L, 
14L, 14L, 14D));
     expectedRangeInfoList.add(
-        new DynamicRangeUtil.DynamicRangeInfo(6, 2766, 32L, 455L, 
163.16666666666666D));
+        new DynamicRangeUtil.DynamicRangeInfo(6, 2766L, 32L, 455L, 
163.16666666666666D));
     assertDynamicNumericRangeResults(values, weights, 4, 55109, 
expectedRangeInfoList);
   }
 
+  public void testComputeDynamicNumericRangesWithLargeTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // More requested ranges (TopN) than values should return ranges with 
weights larger than the
+    // average weight - excluding the last range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 560L, 
277L, 277L, 277D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 508L, 
439L, 439L, 439D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 42, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSingleTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // A single requested range (TopN) should return a single range regardless 
of the weights
+    // provided
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(4, 1863L, 
277L, 794L, 499.25D));
+    assertDynamicNumericRangeResults(values, weights, 1, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithTwoTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // Two requested ranges (TopN) should return two ranges where the first 
range's weight is equal
+    // or larger than half of the total weight
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 1068L, 
277L, 439L, 358.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 2, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameWeights() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[100];
+    long[] weights = new long[100];
+    for (int i = 0; i < 100; i++) {
+      values[i] = i;
+      weights[i] = 50;
+    }
+
+    // Supplying only equal weights should return ranges with equal counts - 
excluding the last
+    // range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
0L, 24L, 12.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
25L, 49L, 37.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
50L, 74L, 62.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
75L, 99L, 87.0D));
+    assertDynamicNumericRangeResults(values, weights, 4, 5000L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameWeightsShuffled() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[100];
+    long[] weights = new long[100];
+    for (int i = 0; i < 100; i++) {
+      values[i] = i;
+      weights[i] = 50;
+    }
+
+    // Shuffling the values and weights should not change the answer when the 
weights are the same
+    shuffleValuesWeights(values, weights);
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
0L, 24L, 12.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
25L, 49L, 37.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
50L, 74L, 62.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(25, 1250L, 
75L, 99L, 87.0D));
+    assertDynamicNumericRangeResults(values, weights, 4, 5000L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameValuesShuffled() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long totalWeight = 0;
+    long[] values = new long[100];
+    long[] weights = new long[100];
+    for (int i = 0; i < 100; i++) {
+      values[i] = 50;
+      weights[i] = i;
+      totalWeight += i;
+    }
+
+    // Shuffling the values and weights should not change the answer when the 
values are the same
+    shuffleValuesWeights(values, weights);
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(51, 1275L, 
50L, 50L, 50D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(21, 1281L, 
50L, 50L, 50D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(16, 1272L, 
50L, 50L, 50D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(12, 1122L, 
50L, 50L, 50D));
+
+    assertDynamicNumericRangeResults(values, weights, 4, totalWeight, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithMisplacedValue() {

Review Comment:
   What does this test reveal that is different from the tests with shuffling?



##########
lucene/facet/src/test/org/apache/lucene/facet/range/TestDynamicRangeUtil.java:
##########
@@ -76,13 +77,248 @@ public void 
testComputeDynamicNumericRangesWithOneLargeWeight() {
     long[] values = new long[] {45, 32, 52, 14, 455, 342, 53};
     long[] weights = new long[] {143, 23, 1, 52343, 53, 12, 2534};
 
-    // value 14 has its own bin since the weight is large, and the rest of 
values fall the other bin
-    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343, 
14L, 14L, 14D));
+    // value 14 has its own bin since the weight is large, and the rest of the 
values fall in the
+    // other bin
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343L, 
14L, 14L, 14D));
     expectedRangeInfoList.add(
-        new DynamicRangeUtil.DynamicRangeInfo(6, 2766, 32L, 455L, 
163.16666666666666D));
+        new DynamicRangeUtil.DynamicRangeInfo(6, 2766L, 32L, 455L, 
163.16666666666666D));
     assertDynamicNumericRangeResults(values, weights, 4, 55109, 
expectedRangeInfoList);
   }
 
+  public void testComputeDynamicNumericRangesWithLargeTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // More requested ranges (TopN) than values should return ranges with 
weights larger than the
+    // average weight - excluding the last range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 560L, 
277L, 277L, 277D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 508L, 
439L, 439L, 439D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 42, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSingleTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // A single requested range (TopN) should return a single range regardless 
of the weights
+    // provided
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(4, 1863L, 
277L, 794L, 499.25D));
+    assertDynamicNumericRangeResults(values, weights, 1, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithTwoTopN() {

Review Comment:
   Does this test catch anything the other tests wouldn't?



##########
lucene/facet/src/test/org/apache/lucene/facet/range/TestDynamicRangeUtil.java:
##########
@@ -76,13 +77,248 @@ public void 
testComputeDynamicNumericRangesWithOneLargeWeight() {
     long[] values = new long[] {45, 32, 52, 14, 455, 342, 53};
     long[] weights = new long[] {143, 23, 1, 52343, 53, 12, 2534};
 
-    // value 14 has its own bin since the weight is large, and the rest of 
values fall the other bin
-    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343, 
14L, 14L, 14D));
+    // value 14 has its own bin since the weight is large, and the rest of the 
values fall in the
+    // other bin
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 52343L, 
14L, 14L, 14D));
     expectedRangeInfoList.add(
-        new DynamicRangeUtil.DynamicRangeInfo(6, 2766, 32L, 455L, 
163.16666666666666D));
+        new DynamicRangeUtil.DynamicRangeInfo(6, 2766L, 32L, 455L, 
163.16666666666666D));
     assertDynamicNumericRangeResults(values, weights, 4, 55109, 
expectedRangeInfoList);
   }
 
+  public void testComputeDynamicNumericRangesWithLargeTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // More requested ranges (TopN) than values should return ranges with 
weights larger than the
+    // average weight - excluding the last range
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 560L, 
277L, 277L, 277D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(1, 508L, 
439L, 439L, 439D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 42, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSingleTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // A single requested range (TopN) should return a single range regardless 
of the weights
+    // provided
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(4, 1863L, 
277L, 794L, 499.25D));
+    assertDynamicNumericRangeResults(values, weights, 1, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithTwoTopN() {
+    List<DynamicRangeUtil.DynamicRangeInfo> expectedRangeInfoList = new 
ArrayList<>();
+    long[] values = new long[] {487, 439, 794, 277};
+    long[] weights = new long[] {59, 508, 736, 560};
+
+    // Two requested ranges (TopN) should return two ranges where the first 
range's weight is equal
+    // or larger than half of the total weight
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 1068L, 
277L, 439L, 358.0D));
+    expectedRangeInfoList.add(new DynamicRangeUtil.DynamicRangeInfo(2, 795L, 
487L, 794L, 640.5D));
+    assertDynamicNumericRangeResults(values, weights, 2, 1863L, 
expectedRangeInfoList);
+  }
+
+  public void testComputeDynamicNumericRangesWithSameWeights() {

Review Comment:
   This is a good test to have! Should we move it next to 
`testComputeDynamicNumericRangesWithSameValues`?



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