Do not allow negative variances (#37384)

Due to floating point error, it was possible for variances to become negative which should never happen.  This bugfix sets variance to zero if it becomes negative as a result of fp error.
This commit is contained in:
Vishnu Gt 2019-01-25 20:26:34 +05:30 committed by Zachary Tong
parent ef8dd12c6d
commit 27c3fb8e0d
5 changed files with 37 additions and 4 deletions

View File

@ -202,7 +202,8 @@ class ExtendedStatsAggregator extends NumericMetricsAggregator.MultiValue {
private double variance(long owningBucketOrd) {
double sum = sums.get(owningBucketOrd);
long count = counts.get(owningBucketOrd);
return (sumOfSqrs.get(owningBucketOrd) - ((sum * sum) / count)) / count;
double variance = (sumOfSqrs.get(owningBucketOrd) - ((sum * sum) / count)) / count;
return variance < 0 ? 0 : variance;
}
@Override

View File

@ -101,7 +101,8 @@ public class InternalExtendedStats extends InternalStats implements ExtendedStat
@Override
public double getVariance() {
return (sumOfSqrs - ((sum * sum) / count)) / count;
double variance = (sumOfSqrs - ((sum * sum) / count)) / count;
return variance < 0 ? 0 : variance;
}
@Override

View File

@ -99,6 +99,34 @@ public class ExtendedStatsAggregatorTests extends AggregatorTestCase {
);
}
/**
* Testcase for https://github.com/elastic/elasticsearch/issues/37303
*/
public void testVarianceNonNegative() throws IOException {
MappedFieldType ft =
new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
ft.setName("field");
final ExtendedSimpleStatsAggregator expected = new ExtendedSimpleStatsAggregator();
testCase(ft,
iw -> {
int numDocs = 3;
for (int i = 0; i < numDocs; i++) {
Document doc = new Document();
double value = 49.95d;
long valueAsLong = NumericUtils.doubleToSortableLong(value);
doc.add(new SortedNumericDocValuesField("field", valueAsLong));
expected.add(value);
iw.addDocument(doc);
}
},
stats -> {
//since the value(49.95) is a constant, variance should be 0
assertEquals(0.0d, stats.getVariance(), TOLERANCE);
assertEquals(0.0d, stats.getStdDeviation(), TOLERANCE);
}
);
}
public void testRandomLongs() throws IOException {
MappedFieldType ft =
new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.LONG);
@ -236,7 +264,8 @@ public class ExtendedStatsAggregatorTests extends AggregatorTestCase {
}
double variance() {
return (sumOfSqrs - ((sum * sum) / count)) / count;
double variance = (sumOfSqrs - ((sum * sum) / count)) / count;
return variance < 0 ? 0 : variance;
}
}
}

View File

@ -73,7 +73,8 @@ public class ExtendedStatsIT extends AbstractNumericTestCase {
sum += val;
sumOfSqrs += val * val;
}
return (sumOfSqrs - ((sum * sum) / vals.length)) / vals.length;
double variance = (sumOfSqrs - ((sum * sum) / vals.length)) / vals.length;
return variance < 0 ? 0 : variance;
}
@Override

View File

@ -137,6 +137,7 @@ public class ExtendedStatsBucketIT extends ESIntegTestCase {
double sumOfSqrs = 1.0 + 1.0 + 1.0 + 4.0 + 0.0 + 1.0;
double avg = sum / count;
double var = (sumOfSqrs - ((sum * sum) / count)) / count;
var = var < 0 ? 0 : var;
double stdDev = Math.sqrt(var);
assertThat(extendedStatsBucketValue, notNullValue());
assertThat(extendedStatsBucketValue.getName(), equalTo("extended_stats_bucket"));