MATH-1671: Reduce the method overrides for the SemiVariance

The SemiVariance had many untested methods and the implementation was
bugged. This change corrects the implementation:

- from using (i=start; i<length; i++) to use i<start+length
- the use of arguments 0 and values.length to start and length for the
array sub-range method

Tests have been added for sub-range evaluation and to complete code
coverage for the class.

Author: aherbert

commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/StatUtils.java

@@ -228,9 +228,6 @@ public static double mean(final double[] values) throws MathIllegalArgumentExcep
      * is empty.
      * <p>
      * Throws <code>IllegalArgumentException</code> if the array is null.
-     * <p>
-     * See {@link org.apache.commons.math4.legacy.stat.descriptive.moment.Mean Mean} for
-     * details on the computing algorithm.
      *
      * @param values the input array
      * @param begin index of the first array element to include

commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/SemiVariance.java

@@ -19,6 +19,7 @@
 
 import org.apache.commons.math4.legacy.exception.MathIllegalArgumentException;
 import org.apache.commons.math4.legacy.exception.NullArgumentException;
+import org.apache.commons.math4.legacy.exception.util.LocalizedFormats;
 import org.apache.commons.math4.legacy.stat.descriptive.AbstractUnivariateStatistic;
 import org.apache.commons.math4.legacy.core.MathArrays;
 
@@ -35,8 +36,7 @@
  *
  * <p>The cutoff value defaults to the mean, bias correction defaults to <code>true</code>
  * and the "variance direction" (upside or downside) defaults to downside.  The variance direction
- * and bias correction may be set using property setters or their values can provided as
- * parameters to {@link #evaluate(double[], double, Direction, boolean, int, int)}.</p>
+ * and bias correction may be set using property setters.</p>
  *
  * <p>If the input array is null, <code>evaluate</code> methods throw
  * <code>IllegalArgumentException.</code>  If the array has length 1, <code>0</code>
@@ -51,18 +51,6 @@
  */
 public class SemiVariance extends AbstractUnivariateStatistic {
 
-    /**
-     * The UPSIDE Direction is used to specify that the observations above the
-     * cutoff point will be used to calculate SemiVariance.
-     */
-    public static final Direction UPSIDE_VARIANCE = Direction.UPSIDE;
-
-    /**
-     * The DOWNSIDE Direction is used to specify that the observations below.
-     * the cutoff point will be used to calculate SemiVariance
-     */
-    public static final Direction DOWNSIDE_VARIANCE = Direction.DOWNSIDE;
-
     /**
      * Determines whether or not bias correction is applied when computing the
      * value of the statistic.  True means that bias is corrected.
@@ -79,56 +67,7 @@ public class SemiVariance extends AbstractUnivariateStatistic {
      * property and default (Downside) <code>varianceDirection</code> property.
      */
     public SemiVariance() {
-    }
-
-    /**
-     * Constructs a SemiVariance with the specified <code>biasCorrected</code>
-     * property and default (Downside) <code>varianceDirection</code> property.
-     *
-     * @param biasCorrected  setting for bias correction - true means
-     * bias will be corrected and is equivalent to using the "no arg"
-     * constructor
-     */
-    public SemiVariance(final boolean biasCorrected) {
-        this.biasCorrected = biasCorrected;
-    }
-
-    /**
-     * Constructs a SemiVariance with the specified <code>Direction</code> property.
-     * and default (true) <code>biasCorrected</code> property
-     *
-     * @param direction  setting for the direction of the SemiVariance
-     * to calculate
-     */
-    public SemiVariance(final Direction direction) {
-        this.varianceDirection = direction;
-    }
-
-    /**
-     * Constructs a SemiVariance with the specified <code>isBiasCorrected</code>
-     * property and the specified <code>Direction</code> property.
-     *
-     * @param corrected  setting for bias correction - true means
-     * bias will be corrected and is equivalent to using the "no arg"
-     * constructor
-     *
-     * @param direction  setting for the direction of the SemiVariance
-     * to calculate
-     */
-    public SemiVariance(final boolean corrected, final Direction direction) {
-        this.biasCorrected = corrected;
-        this.varianceDirection = direction;
-    }
-
-    /**
-     * Copy constructor, creates a new {@code SemiVariance} identical
-     * to the {@code original}.
-     *
-     * @param original the {@code SemiVariance} instance to copy
-     * @throws NullArgumentException  if original is null
-     */
-    public SemiVariance(final SemiVariance original) throws NullArgumentException {
-        copy(original, this);
+        // Do nothing
     }
 
     /**
@@ -137,27 +76,11 @@ public SemiVariance(final SemiVariance original) throws NullArgumentException {
     @Override
     public SemiVariance copy() {
         SemiVariance result = new SemiVariance();
-        // No try-catch or advertised exception because args are guaranteed non-null
-        copy(this, result);
+        result.biasCorrected = biasCorrected;
+        result.varianceDirection = varianceDirection;
         return result;
     }
 
-    /**
-     * Copies source to dest.
-     * <p>Neither source nor dest can be null.</p>
-     *
-     * @param source SemiVariance to copy
-     * @param dest SemiVariance to copy to
-     * @throws NullArgumentException if either source or dest is null
-     */
-    public static void copy(final SemiVariance source, SemiVariance dest)
-        throws NullArgumentException {
-        NullArgumentException.check(source);
-        NullArgumentException.check(dest);
-        dest.biasCorrected = source.biasCorrected;
-        dest.varianceDirection = source.varianceDirection;
-    }
-
     /**
      * <p>Returns the {@link SemiVariance} of the designated values against the mean, using
      * instance properties varianceDirection and biasCorrection.</p>
@@ -177,24 +100,7 @@ public double evaluate(final double[] values, final int start, final int length)
          throws MathIllegalArgumentException {
          MathArrays.verifyValues(values, start, length);
          double m = org.apache.commons.statistics.descriptive.Mean.ofRange(values, start, start + length).getAsDouble();
-         return evaluate(values, m, varianceDirection, biasCorrected, 0, values.length);
-     }
-
-     /**
-      * This method calculates {@link SemiVariance} for the entire array against the mean, using
-      * the current value of the biasCorrection instance property.
-      *
-      * @param values the input array
-      * @param direction the {@link Direction} of the semivariance
-      * @return the SemiVariance
-      * @throws MathIllegalArgumentException if values is null
-      *
-      */
-     public double evaluate(final double[] values, Direction direction)
-         throws MathIllegalArgumentException {
-         MathArrays.verifyValues(values, 0, 0);
-         double m = org.apache.commons.statistics.descriptive.Mean.of(values).getAsDouble();
-         return evaluate(values, m, direction, biasCorrected, 0, values.length);
+         return compute(values, m, varianceDirection, biasCorrected, start, length);
      }
 
      /**
@@ -211,33 +117,38 @@ public double evaluate(final double[] values, Direction direction)
       */
      public double evaluate(final double[] values, final double cutoff)
          throws MathIllegalArgumentException {
-         return evaluate(values, cutoff, varianceDirection, biasCorrected, 0, values.length);
+         if (values == null) {
+             throw new NullArgumentException(LocalizedFormats.INPUT_ARRAY);
+         }
+         return compute(values, cutoff, varianceDirection, biasCorrected, 0, values.length);
      }
 
      /**
-      * <p>Returns the {@link SemiVariance} of the designated values against the cutoff in the
-      * given direction, using the current value of the biasCorrection instance property.</p>
+      * <p>Returns the {@link SemiVariance} of the designated values against the cutoff
+      * in the given direction with the provided bias correction.</p>
       *
       * <p>Returns <code>NaN</code> if the array is empty and throws
-      * <code>MathIllegalArgumentException</code> if the array is null.</p>
+      * <code>IllegalArgumentException</code> if the array is null.</p>
       *
       * @param values the input array
       * @param cutoff the reference point
-      * @param direction the {@link Direction} of the semivariance
+      * @param start index of the first array element to include
+      * @param length the number of elements to include
       * @return the SemiVariance
-      * @throws MathIllegalArgumentException if values is null
+      * @throws MathIllegalArgumentException if the parameters are not valid
+      *
       */
-     public double evaluate(final double[] values, final double cutoff, final Direction direction)
+     public double evaluate(final double[] values, final double cutoff, final int start, final int length)
          throws MathIllegalArgumentException {
-         return evaluate(values, cutoff, direction, biasCorrected, 0, values.length);
+         MathArrays.verifyValues(values, start, length);
+         return compute(values, cutoff, varianceDirection, biasCorrected, start, length);
      }
 
      /**
       * <p>Returns the {@link SemiVariance} of the designated values against the cutoff
       * in the given direction with the provided bias correction.</p>
       *
-      * <p>Returns <code>NaN</code> if the array is empty and throws
-      * <code>IllegalArgumentException</code> if the array is null.</p>
+      * <p>Returns <code>NaN</code> if the array is empty.</p>
       *
       * @param values the input array
       * @param cutoff the reference point
@@ -246,38 +157,33 @@ public double evaluate(final double[] values, final double cutoff, final Directi
       * @param start index of the first array element to include
       * @param length the number of elements to include
       * @return the SemiVariance
-      * @throws MathIllegalArgumentException if the parameters are not valid
-      *
       */
-     public double evaluate (final double[] values, final double cutoff, final Direction direction,
-                             final boolean corrected, final int start, final int length)
-         throws MathIllegalArgumentException {
-
-         MathArrays.verifyValues(values, start, length);
-         if (values.length == 0) {
+     private static double compute(double[] values, double cutoff, Direction direction,
+             boolean corrected, int start, int length) {
+         // Arguments must have been validated
+         if (length == 0) {
              return Double.NaN;
-         } else {
-             if (values.length == 1) {
-                 return 0.0;
-             } else {
-                 final boolean booleanDirection = direction.getDirection();
-
-                 double dev = 0.0;
-                 double sumsq = 0.0;
-                 for (int i = start; i < length; i++) {
-                     if ((values[i] > cutoff) == booleanDirection) {
-                         dev = values[i] - cutoff;
-                         sumsq += dev * dev;
-                     }
-                 }
+         }
+         if (length == 1) {
+             return 0.0;
+         }
+         final boolean booleanDirection = direction.getDirection();
 
-                 if (corrected) {
-                     return sumsq / (length - 1.0);
-                 } else {
-                     return sumsq / length;
-                 }
+         double dev = 0.0;
+         double sumsq = 0.0;
+         final int end = start + length;
+         for (int i = start; i < end; i++) {
+             if ((values[i] > cutoff) == booleanDirection) {
+                 dev = values[i] - cutoff;
+                 sumsq += dev * dev;
              }
          }
+
+         if (corrected) {
+             return sumsq / (length - 1.0);
+         } else {
+             return sumsq / length;
+         }
      }
 
      /**

…legacy/stat/descriptive/moment/Mean.java …tat/descriptive/moment/WeightedMean.javacommons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Mean.java renamed to commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/WeightedMean.java commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Mean.java renamed to commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/WeightedMean.java

@@ -21,12 +21,11 @@
 import org.apache.commons.math4.legacy.stat.descriptive.WeightedEvaluation;
 
 /**
- * Computes the arithmetic mean of a set of values. Uses the definitional
- * formula:
+ * Computes the weighted mean of a set of values. Uses the formula:
  * <p>
- * mean = sum(x_i) / n
+ * mean = sum(w_i * x_i) / sum(w_i)
  * </p>
- * <p>where <code>n</code> is the number of observations.
+ * <p>where <code>w_i</code> is the weight for observation <code>x_i</code>.
  * </p>
  * <p> If used to compute the mean of an array
  * of stored values, a two-pass, corrected algorithm is used, starting with
@@ -41,21 +40,32 @@
  *  values.
  * </p>
  */
-public class Mean implements WeightedEvaluation {
+public final class WeightedMean implements WeightedEvaluation {
+    /** An instance. */
+    private static final WeightedMean INSTANCE = new WeightedMean();
 
-    /** Constructs a Mean. */
-    public Mean() {
+    /** Create an instance. */
+    private WeightedMean() {
         // Do nothing
     }
 
+    /**
+     * Gets an instance.
+     *
+     * @return an instance
+     */
+    public static WeightedMean getInstance() {
+        return INSTANCE;
+    }
+
     /**
      * Returns the weighted arithmetic mean of the entries in the specified portion of
      * the input array, or <code>Double.NaN</code> if the designated subarray
      * is empty.
      * <p>
      * Throws <code>IllegalArgumentException</code> if either array is null.</p>
      * <p>
-     * See {@link Mean} for details on the computing algorithm. The two-pass algorithm
+     * See {@link WeightedMean} for details on the computing algorithm. The two-pass algorithm
      * described above is used here, with weights applied in computing both the original
      * estimate and the correction factor.</p>
      * <p>
@@ -102,7 +112,7 @@ public double evaluate(final double[] values, final double[] weights,
      * <p>
      * Throws <code>MathIllegalArgumentException</code> if either array is null.</p>
      * <p>
-     * See {@link Mean} for details on the computing algorithm. The two-pass algorithm
+     * See {@link WeightedMean} for details on the computing algorithm. The two-pass algorithm
      * described above is used here, with weights applied in computing both the original
      * estimate and the correction factor.</p>
      * <p>

…cy/stat/descriptive/moment/Variance.java …descriptive/moment/WeightedVariance.javacommons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java renamed to commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/WeightedVariance.java commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/Variance.java renamed to commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/stat/descriptive/moment/WeightedVariance.java

@@ -21,37 +21,45 @@
 import org.apache.commons.math4.legacy.stat.descriptive.WeightedEvaluation;
 
 /**
- * Computes the variance of the available values.  By default, the unbiased
+ * Computes the weighted variance of the available values.  By default, the unbiased
  * "sample variance" definitional formula is used:
  * <p>
- * variance = sum((x_i - mean)^2) / (n - 1) </p>
+ * variance = sum(w_i * (x_i - mean)^2) / (sum(w_i) - 1) </p>
  * <p>
- * where mean is the {@link Mean} and <code>n</code> is the number
- * of sample observations.</p>
+ * where mean is the {@link WeightedMean} and <code>w_i</code> is the weight for
+ * observation <code>x_i</code>.</p>
  * <p>
- * The "population variance"  ( sum((x_i - mean)^2) / n ) can also
+ * The "population variance" using the denominator as <code>sum(w_i)</code> can also
  * be computed using this statistic.  The <code>isBiasCorrected</code>
  * property determines whether the "population" or "sample" value is
  * returned by the <code>evaluate</code> methods.
  * To compute population variances, set this property to <code>false.</code>
  * </p>
- * <p>
-Ø */
-public class Variance implements WeightedEvaluation {
+ */
+public final class WeightedVariance implements WeightedEvaluation {
     /**
      * Whether or not bias correction is applied when computing the
      * value of the statistic. True means that bias is corrected.  See
-     * {@link Variance} for details on the formula.
+     * {@link WeightedVariance} for details on the formula.
      */
     private boolean isBiasCorrected = true;
 
     /**
      * Constructs a Variance.
      */
-    public Variance() {
+    private WeightedVariance() {
         // Do nothing
     }
 
+    /**
+     * Gets a new instance.
+     *
+     * @return an instance
+     */
+    public static WeightedVariance getInstance() {
+        return new WeightedVariance();
+    }
+
     /**
      * <p>Returns the weighted variance of the entries in the specified portion of
      * the input array, or <code>Double.NaN</code> if the designated subarray
@@ -105,7 +113,7 @@ public double evaluate(final double[] values, final double[] weights,
             if (length == 1) {
                 var = 0.0;
             } else if (length > 1) {
-                Mean mean = new Mean();
+                WeightedMean mean = WeightedMean.getInstance();
                 double m = mean.evaluate(values, weights, begin, length);
                 var = evaluate(values, weights, m, begin, length);
             }