This is an automated email from the ASF dual-hosted git repository.

erans pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-math.git

commit bdb4160b004d997df9494ab18a580dd11b4eda47
Author: Alex Herbert <aherb...@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));
+    }
+
 }

Reply via email to