You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ah...@apache.org on 2021/08/21 09:04:52 UTC
[commons-math] branch master updated (3f7de0e -> 569e35d)
This is an automated email from the ASF dual-hosted git repository.
aherbert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-math.git.
from 3f7de0e sonar fix: Handle variance=0 in skewness computation for input array
new a2efd4f Correct verification of zero length values and weights
new 569e35d Add variance tests for zero weights
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../commons/math4/legacy/core/MathArrays.java | 42 ++++++++++++----------
.../commons/math4/legacy/core/MathArraysTest.java | 9 +++++
.../legacy/stat/descriptive/moment/Variance.java | 10 +++++-
.../stat/descriptive/moment/VarianceTest.java | 27 ++++++++++++++
4 files changed, 68 insertions(+), 20 deletions(-)
[commons-math] 02/02: Add variance tests for zero weights
Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-math.git
commit 569e35ddbe98ed5dc74c74dbc2129d9178b818a1
Author: Alex Herbert <ah...@apache.org>
AuthorDate: Sat Aug 21 10:04:49 2021 +0100
Add variance tests for zero weights
Update javadoc for the behaviour when input weights are zero.
This issue was found when checking the sonar report for the variance
class which has a potential divide by zero if the weights sum to zero.
---
.../legacy/stat/descriptive/moment/Variance.java | 10 +++++++-
.../stat/descriptive/moment/VarianceTest.java | 27 ++++++++++++++++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java
index 9c235da..415bbb1 100644
--- a/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java
+++ b/commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java
@@ -297,6 +297,7 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* <li>the start and length arguments do not determine a valid array</li>
* </ul>
* <p>
@@ -318,7 +319,7 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
double var = Double.NaN;
- if (MathArrays.verifyValues(values, weights,begin, length)) {
+ if (MathArrays.verifyValues(values, weights, begin, length)) {
if (length == 1) {
var = 0.0;
} else if (length > 1) {
@@ -356,6 +357,7 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* </ul>
* <p>
* Does not change the internal state of the statistic.</p>
@@ -488,6 +490,7 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* <li>the start and length arguments do not determine a valid array</li>
* </ul>
* <p>
@@ -527,6 +530,10 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
}
if (isBiasCorrected) {
+ // Note: For this to be valid the weights should correspond to counts
+ // of each observation where the weights are positive integers; the
+ // sum of the weights is the total number of observations and should
+ // be at least 2.
var = (accum - (accum2 * accum2 / sumWts)) / (sumWts - 1.0);
} else {
var = (accum - (accum2 * accum2 / sumWts)) / sumWts;
@@ -566,6 +573,7 @@ public class Variance extends AbstractStorelessUnivariateStatistic implements Se
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* </ul>
* <p>
* Does not change the internal state of the statistic.</p>
diff --git a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/moment/VarianceTest.java b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/moment/VarianceTest.java
index a3261af..bd7228d 100644
--- a/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/moment/VarianceTest.java
+++ b/commons-math-legacy/src/test/java/org/apache/commons/math4/legacy/stat/descriptive/moment/VarianceTest.java
@@ -19,8 +19,10 @@ package org.apache.commons.math4.legacy.stat.descriptive.moment;
import org.apache.commons.math4.legacy.stat.descriptive.StorelessUnivariateStatisticAbstractTest;
import org.apache.commons.math4.legacy.stat.descriptive.UnivariateStatistic;
import org.apache.commons.math4.legacy.core.MathArrays;
+import org.apache.commons.math4.legacy.exception.MathIllegalArgumentException;
import org.junit.Assert;
import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
/**
* Test cases for the {@link UnivariateStatistic} class.
@@ -114,4 +116,29 @@ public class VarianceTest extends StorelessUnivariateStatisticAbstractTest{
}
+ @Test
+ public void testZeroWeights() {
+ Variance variance = new Variance();
+ final double[] values = {1, 2, 3, 4};
+ final double[] weights = new double[values.length];
+
+ // No weights
+ Assertions.assertThrows(MathIllegalArgumentException.class, () -> {
+ variance.evaluate(values, weights);
+ });
+
+ // No length
+ final int begin = 1;
+ final int zeroLength = 0;
+ Assertions.assertEquals(Double.NaN, variance.evaluate(values, weights, begin, zeroLength));
+
+ // One weight (must be non-zero)
+ Assertions.assertThrows(MathIllegalArgumentException.class, () -> {
+ variance.evaluate(values, weights, begin, zeroLength + 1);
+ });
+
+ weights[begin] = Double.MIN_VALUE;
+ Assertions.assertEquals(0.0, variance.evaluate(values, weights, begin, zeroLength + 1));
+ }
+
}
[commons-math] 01/02: Correct verification of zero length values
and weights
Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-math.git
commit a2efd4fad5498b2f8b42b75c07f5bd96caba0b66
Author: Alex Herbert <ah...@apache.org>
AuthorDate: Sat Aug 21 09:58:13 2021 +0100
Correct verification of zero length values and weights
This bug was found when checking the sonar report for the variance class
which uses MathArrays.verifyValues.
---
.../commons/math4/legacy/core/MathArrays.java | 42 ++++++++++++----------
.../commons/math4/legacy/core/MathArraysTest.java | 9 +++++
2 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/commons-math-legacy-core/src/main/java/org/apache/commons/math4/legacy/core/MathArrays.java b/commons-math-legacy-core/src/main/java/org/apache/commons/math4/legacy/core/MathArrays.java
index 00d3c48..d92362b 100644
--- a/commons-math-legacy-core/src/main/java/org/apache/commons/math4/legacy/core/MathArrays.java
+++ b/commons-math-legacy-core/src/main/java/org/apache/commons/math4/legacy/core/MathArrays.java
@@ -968,6 +968,7 @@ public final class MathArrays {
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* <li>the start and length arguments do not determine a valid array</li></ul>
* </li>
* <li>returns <code>false</code> if the array is non-null, but
@@ -1004,6 +1005,7 @@ public final class MathArrays {
* <li>the weights array contains one or more infinite values</li>
* <li>the weights array contains one or more NaN values</li>
* <li>the weights array contains negative values</li>
+ * <li>the weights array does not contain at least one non-zero value (applies when length is non zero)</li>
* <li>the start and length arguments do not determine a valid array</li></ul>
* </li>
* <li>returns <code>false</code> if the array is non-null, but
@@ -1031,27 +1033,29 @@ public final class MathArrays {
checkEqualLength(weights, values);
- boolean containsPositiveWeight = false;
- for (int i = begin; i < begin + length; i++) {
- final double weight = weights[i];
- if (Double.isNaN(weight)) {
- throw new MathIllegalArgumentException(LocalizedFormats.NAN_ELEMENT_AT_INDEX, Integer.valueOf(i));
- }
- if (Double.isInfinite(weight)) {
- throw new MathIllegalArgumentException(LocalizedFormats.INFINITE_ARRAY_ELEMENT,
- Double.valueOf(weight), Integer.valueOf(i));
- }
- if (weight < 0) {
- throw new MathIllegalArgumentException(LocalizedFormats.NEGATIVE_ELEMENT_AT_INDEX,
- Integer.valueOf(i), Double.valueOf(weight));
- }
- if (!containsPositiveWeight && weight > 0.0) {
- containsPositiveWeight = true;
+ if (length != 0) {
+ boolean containsPositiveWeight = false;
+ for (int i = begin; i < begin + length; i++) {
+ final double weight = weights[i];
+ if (Double.isNaN(weight)) {
+ throw new MathIllegalArgumentException(LocalizedFormats.NAN_ELEMENT_AT_INDEX, Integer.valueOf(i));
+ }
+ if (Double.isInfinite(weight)) {
+ throw new MathIllegalArgumentException(LocalizedFormats.INFINITE_ARRAY_ELEMENT,
+ Double.valueOf(weight), Integer.valueOf(i));
+ }
+ if (weight < 0) {
+ throw new MathIllegalArgumentException(LocalizedFormats.NEGATIVE_ELEMENT_AT_INDEX,
+ Integer.valueOf(i), Double.valueOf(weight));
+ }
+ if (!containsPositiveWeight && weight > 0.0) {
+ containsPositiveWeight = true;
+ }
}
- }
- if (!containsPositiveWeight) {
- throw new MathIllegalArgumentException(LocalizedFormats.WEIGHT_AT_LEAST_ONE_NON_ZERO);
+ if (!containsPositiveWeight) {
+ throw new MathIllegalArgumentException(LocalizedFormats.WEIGHT_AT_LEAST_ONE_NON_ZERO);
+ }
}
return verifyValues(values, begin, length, allowEmpty);
diff --git a/commons-math-legacy-core/src/test/java/org/apache/commons/math4/legacy/core/MathArraysTest.java b/commons-math-legacy-core/src/test/java/org/apache/commons/math4/legacy/core/MathArraysTest.java
index 84b7fc5..aa71a74 100644
--- a/commons-math-legacy-core/src/test/java/org/apache/commons/math4/legacy/core/MathArraysTest.java
+++ b/commons-math-legacy-core/src/test/java/org/apache/commons/math4/legacy/core/MathArraysTest.java
@@ -635,6 +635,9 @@ public class MathArraysTest {
final double[] nullArray = null;
Assert.assertFalse(MathArrays.verifyValues(singletonArray, 0, 0));
Assert.assertFalse(MathArrays.verifyValues(testArray, 0, 0));
+ Assert.assertTrue(MathArrays.verifyValues(testArray, 0, 0, true));
+ Assert.assertFalse(MathArrays.verifyValues(testArray, testWeightsArray, 0, 0));
+ Assert.assertTrue(MathArrays.verifyValues(testArray, testWeightsArray, 0, 0, true));
try {
MathArrays.verifyValues(singletonArray, 2, 1); // start past end
Assert.fail("Expecting MathIllegalArgumentException");
@@ -683,6 +686,12 @@ public class MathArraysTest {
} catch (MathIllegalArgumentException ex) {
// expected
}
+ try {
+ MathArrays.verifyValues(testArray, new double[testArray.length], 0, 6); // can't have all zero weights
+ Assert.fail("Expecting MathIllegalArgumentException");
+ } catch (MathIllegalArgumentException ex) {
+ // expected
+ }
}
@Test