goiri commented on code in PR #5495:
URL: https://github.com/apache/hadoop/pull/5495#discussion_r1152443921
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -92,27 +93,68 @@ public void testClear() throws IOException {
public void testQuantileError() throws IOException {
final int count = 100000;
Random r = new Random(0xDEADDEAD);
Review Comment:
Where are we using this random?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -92,27 +93,68 @@ public void testClear() throws IOException {
public void testQuantileError() throws IOException {
final int count = 100000;
Random r = new Random(0xDEADDEAD);
- Long[] values = new Long[count];
+ int[] values = new int[count];
for (int i = 0; i < count; i++) {
- values[i] = (long) (i + 1);
+ values[i] = i + 1;
}
+
// Do 10 shuffle/insert/check cycles
for (int i = 0; i < 10; i++) {
- System.out.println("Starting run " + i);
+
+ // Shuffle
Collections.shuffle(Arrays.asList(values), r);
estimator.clear();
+
+ // Insert
for (int j = 0; j < count; j++) {
estimator.insert(values[j]);
}
Map<Quantile, Long> snapshot;
snapshot = estimator.snapshot();
+
+ // Check
for (Quantile q : quantiles) {
long actual = (long) (q.quantile * count);
long error = (long) (q.error * count);
long estimate = snapshot.get(q);
- System.out
- .println(String.format("Expected %d with error %d, estimated %d",
- actual, error, estimate));
+ assertThat(estimate <= actual + error).isTrue();
+ assertThat(estimate >= actual - error).isTrue();
+ }
+ }
+ }
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
+ int[] values = new int[count];
+ for (int i = 0; i < count; i++) {
+ values[i] = i + 1;
+ }
+
+ // Do 10 shuffle/insert/check cycles
+ for (int i = 0; i < 10; i++) {
Review Comment:
Make 10 a constant just to show is NUM_REPEATS or something like that.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -92,27 +93,68 @@ public void testClear() throws IOException {
public void testQuantileError() throws IOException {
final int count = 100000;
Random r = new Random(0xDEADDEAD);
Review Comment:
OK, is the shuffle.
Hard to search single letter vars, make it `rnd`.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -92,27 +93,68 @@ public void testClear() throws IOException {
public void testQuantileError() throws IOException {
final int count = 100000;
Random r = new Random(0xDEADDEAD);
- Long[] values = new Long[count];
+ int[] values = new int[count];
for (int i = 0; i < count; i++) {
- values[i] = (long) (i + 1);
+ values[i] = i + 1;
}
+
// Do 10 shuffle/insert/check cycles
for (int i = 0; i < 10; i++) {
- System.out.println("Starting run " + i);
+
+ // Shuffle
Collections.shuffle(Arrays.asList(values), r);
estimator.clear();
+
+ // Insert
for (int j = 0; j < count; j++) {
estimator.insert(values[j]);
}
Map<Quantile, Long> snapshot;
snapshot = estimator.snapshot();
+
+ // Check
for (Quantile q : quantiles) {
long actual = (long) (q.quantile * count);
long error = (long) (q.error * count);
long estimate = snapshot.get(q);
- System.out
- .println(String.format("Expected %d with error %d, estimated %d",
- actual, error, estimate));
+ assertThat(estimate <= actual + error).isTrue();
+ assertThat(estimate >= actual - error).isTrue();
+ }
+ }
+ }
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
+ int[] values = new int[count];
+ for (int i = 0; i < count; i++) {
+ values[i] = i + 1;
+ }
+
+ // Do 10 shuffle/insert/check cycles
+ for (int i = 0; i < 10; i++) {
+ // Shuffle
+ Collections.shuffle(Arrays.asList(values), r);
+ inverseQuantilesEstimator.clear();
+
+ // Insert
+ for (int j = 0; j < count; j++) {
Review Comment:
```
for (int value : values) {
inverseQuantilesEstimator.insert(value);
}
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -92,27 +93,68 @@ public void testClear() throws IOException {
public void testQuantileError() throws IOException {
final int count = 100000;
Random r = new Random(0xDEADDEAD);
- Long[] values = new Long[count];
+ int[] values = new int[count];
for (int i = 0; i < count; i++) {
- values[i] = (long) (i + 1);
+ values[i] = i + 1;
}
+
// Do 10 shuffle/insert/check cycles
for (int i = 0; i < 10; i++) {
- System.out.println("Starting run " + i);
+
+ // Shuffle
Collections.shuffle(Arrays.asList(values), r);
estimator.clear();
+
+ // Insert
for (int j = 0; j < count; j++) {
Review Comment:
As we are at cleaning:
```
for (int value : values) {
estimator.insert(value);
}
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/metrics2/util/TestSampleQuantiles.java:
##########
@@ -118,4 +119,40 @@ public void testQuantileError() throws IOException {
}
}
}
+
+ /**
+ * Correctness test that checks that absolute error of the estimate for
inverse quantiles
+ * is within specified error bounds for some randomly permuted streams of
items.
+ */
+ @Test
+ public void testInverseQuantiles() throws IOException {
+ SampleQuantiles inverseQuantilesEstimator = new
SampleQuantiles(MutableInverseQuantiles.INVERSE_QUANTILES);
+ final int count = 100000;
+ Random r = new Random(0xDEADDEAD);
Review Comment:
Make it `rnd`; it is hard to find.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
