Merge a83fe99 into 85b31f7

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/AbstractContinuousDistribution.java

@@ -36,6 +36,42 @@ abstract class AbstractContinuousDistribution
     // XXX Values copied from defaults in class
     // "o.a.c.math4.analysis.solvers.BaseAbstractUnivariateSolver"
 
+    // Note:
+    // Probability distributions may have very small CDF values.
+    // This creates issues when using the Brent solver.
+    //
+    // 1. It cannot identify bracketing if the multiplication of the two
+    // end points creates a signed zero since the condition uses:
+    //   lower * upper < 0
+    // It should be changed to Double.compare(lower * upper, 0.0) < 0.
+    //
+    // 2. Function value accuracy determines if the Brent solver performs a
+    // search. Ideally set to zero to force a search (unless one of the the
+    // initial bracket values is correct). This can result in functions
+    // that evaluate very small p to raise a no-bracket exception due to [1].
+    //
+    // 3. Solver absolute accuracy is the minimum absolute difference between
+    // bracketing points to continue the search. To invert very small probability
+    // values (which map to very small x values) requires a very small absolute
+    // accuracy otherwise the search stops too soon. Set to zero to force
+    // stopping criteria based only on the relative difference between points.
+    //
+    // Note:
+    // The Brent solver does not allow a stopping criteria for the proximity
+    // to the root. It is hard coded to 1 ULP (CDF(x) - p == 0). Thus we
+    // search until there is a small relative difference between the upper
+    // and lower bracket of the root.
+
+    // TODO:
+    // Extract the Brent solver code into this class and fix it for the known
+    // issues. These can be transferred back into the original class when the
+    // best solution is known.
+    //
+    // Changes to this class affect many samplers. Altering the accuracy
+    // thresholds can cause any test that uses the inverse CDF to fail. This
+    // includes sampling tests for distributions with use the inverse
+    // transform sampler which have been coded with fixed seeds that work.
+
     /** BrentSolver relative accuracy. */
     private static final double SOLVER_RELATIVE_ACCURACY = 1e-14;
     /** BrentSolver absolute accuracy. */
@@ -54,11 +90,12 @@ abstract class AbstractContinuousDistribution
      *     {@link #cumulativeProbability(double) cdf(x) - p}. The bounds may be bracketed for
      *     efficiency.</li>
      * </ul>
+     *
+     * @throws IllegalArgumentException if {@code p < 0} or {@code p > 1}
      */
     @Override
     public double inverseCumulativeProbability(final double p) {
-        /*
-         * IMPLEMENTATION NOTES
+        /* IMPLEMENTATION NOTES
          * --------------------
          * Where applicable, use is made of the one-sided Chebyshev inequality
          * to bracket the root. This inequality states that
@@ -85,10 +122,7 @@ public double inverseCumulativeProbability(final double p) {
          * progressions 1, 2, 4, ... and -1, -2, -4, ... are used to bracket
          * the root.
          */
-        if (p < 0 ||
-            p > 1) {
-            throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
-        }
+        ArgumentUtils.checkProbability(p);
 
         double lowerBound = getSupportLowerBound();
         if (p == 0) {
@@ -103,13 +137,15 @@ public double inverseCumulativeProbability(final double p) {
         final double mu = getMean();
         final double sig = Math.sqrt(getVariance());
         final boolean chebyshevApplies = Double.isFinite(mu) &&
-                                         Double.isFinite(sig);
+                                         ArgumentUtils.isFiniteStrictlyPositive(sig);
 
         if (lowerBound == Double.NEGATIVE_INFINITY) {
             if (chebyshevApplies) {
                 lowerBound = mu - sig * Math.sqrt((1 - p) / p);
-            } else {
-                lowerBound = -1;
+            }
+            // Bound may have been set as infinite
+            if (lowerBound == Double.NEGATIVE_INFINITY) {
+                lowerBound = Math.min(-1, upperBound);
                 while (cumulativeProbability(lowerBound) >= p) {
                     lowerBound *= 2;
                 }
@@ -119,8 +155,10 @@ public double inverseCumulativeProbability(final double p) {
         if (upperBound == Double.POSITIVE_INFINITY) {
             if (chebyshevApplies) {
                 upperBound = mu + sig * Math.sqrt(p / (1 - p));
-            } else {
-                upperBound = 1;
+            }
+            // Bound may have been set as infinite
+            if (upperBound == Double.POSITIVE_INFINITY) {
+                upperBound = Math.max(1, lowerBound);
                 while (cumulativeProbability(upperBound) < p) {
                     upperBound *= 2;
                 }
@@ -138,7 +176,7 @@ public double inverseCumulativeProbability(final double p) {
         if (!isSupportConnected()) {
             /* Test for plateau. */
             final double dx = SOLVER_ABSOLUTE_ACCURACY;
-            if (x - dx >= getSupportLowerBound()) {
+            if (x - dx >= lowerBound) {
                 final double px = cumulativeProbability(x);
                 if (cumulativeProbability(x - dx) == px) {
                     upperBound = x;

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/AbstractDiscreteDistribution.java

@@ -38,13 +38,12 @@ abstract class AbstractDiscreteDistribution
      *     {@link #cumulativeProbability(int)}. The bounds may be bracketed for
      *     efficiency.</li>
      * </ul>
+     *
+     * @throws IllegalArgumentException if {@code p < 0} or {@code p > 1}
      */
     @Override
     public int inverseCumulativeProbability(final double p) {
-        if (p < 0 ||
-            p > 1) {
-            throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
-        }
+        ArgumentUtils.checkProbability(p);
 
         int lower = getSupportLowerBound();
         if (p == 0) {

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/ArgumentUtils.java

@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.distribution;
+
+/**
+ * Utilities for argument validation.
+ */
+final class ArgumentUtils {
+    /** No instances. */
+    private ArgumentUtils() {}
+
+    /**
+     * Checks if the value {@code x} is finite and strictly positive.
+     *
+     * @param x Value
+     * @return true if {@code x > 0} and is finite
+     */
+    static boolean isFiniteStrictlyPositive(double x) {
+        return x > 0 && x < Double.POSITIVE_INFINITY;
+    }
+
+    /**
+     * Check the probability {@code p} is in the interval {@code [0, 1]}.
+     *
+     * @param p Probability
+     * @throws IllegalArgumentException if {@code p < 0} or {@code p > 1}
+     */
+    static void checkProbability(double p) {
+        if (p >= 0 && p <= 1) {
+            return;
+        }
+        // Out-of-range or NaN
+        throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
+    }
+}

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/BetaDistribution.java

@@ -23,6 +23,20 @@
 
 /**
  * Implementation of the <a href="http://en.wikipedia.org/wiki/Beta_distribution">Beta distribution</a>.
+ *
+ * <p>The probability density function of \( X \) is:
+ *
+ * <p>\[ f(x; \alpha, \beta) = \frac{1}{ B(\alpha, \beta)} x^{\alpha-1} (1-x)^{\beta-1} \]
+ *
+ * <p>for \( \alpha &gt; 0 \), \( \beta &gt; 0 \), \( x \in [0, 1] \) and
+ * the beta function, \( B \), is a normalization constant:
+ *
+ * <p>\[ B(\alpha, \beta) = \frac{\Gamma(\alpha+\beta)}{\Gamma(\alpha) \Gamma(\beta)} \]
+ *
+ * <p>where \( \Gamma \) is the gamma function.
+ *
+ * <p>
+ * \( \alpha \) and \( \beta \) are <em>shape</em> parameters.
  */
 public class BetaDistribution extends AbstractContinuousDistribution {
     /** First shape parameter. */
@@ -40,6 +54,13 @@ public class BetaDistribution extends AbstractContinuousDistribution {
      */
     public BetaDistribution(double alpha,
                             double beta) {
+        if (alpha <= 0) {
+            throw new DistributionException(DistributionException.NOT_STRICTLY_POSITIVE, alpha);
+        }
+        if (beta <= 0) {
+            throw new DistributionException(DistributionException.NOT_STRICTLY_POSITIVE, beta);
+        }
+
         this.alpha = alpha;
         this.beta = beta;
         z = LogGamma.value(alpha) + LogGamma.value(beta) - LogGamma.value(alpha + beta);
@@ -63,28 +84,48 @@ public double getBeta() {
         return beta;
     }
 
-    /** {@inheritDoc} */
+    /** {@inheritDoc}
+     *
+     * <p>The density is not defined when {@code x = 0, alpha < 1}, or {@code x = 1, beta < 1}.
+     * In this case the limit of infinity is returned.
+     */
     @Override
     public double density(double x) {
         return Math.exp(logDensity(x));
     }
 
-    /** {@inheritDoc} **/
+    /** {@inheritDoc}
+     *
+     * <p>The density is not defined when {@code x = 0, alpha < 1}, or {@code x = 1, beta < 1}.
+     * In this case the limit of infinity is returned.
+     */
     @Override
     public double logDensity(double x) {
         if (x < 0 ||
             x > 1) {
             return Double.NEGATIVE_INFINITY;
         } else if (x == 0) {
             if (alpha < 1) {
-                throw new DistributionException(DistributionException.TOO_SMALL,
-                                                alpha, 1.0);
+                // Distribution is not valid when x=0, alpha<1
+                // due to a divide by zero error.
+                // Do not raise an exception and return the limit.
+                return Double.POSITIVE_INFINITY;
+            }
+            // Special case of cancellation: x^(a-1) (1-x)^(b-1) / B(a, b)
+            if (alpha == 1) {
+                return -z;
             }
             return Double.NEGATIVE_INFINITY;
         } else if (x == 1) {
             if (beta < 1) {
-                throw new DistributionException(DistributionException.TOO_SMALL,
-                                                beta, 1.0);
+                // Distribution is not valid when x=1, beta<1
+                // due to a divide by zero error.
+                // Do not raise an exception and return the limit.
+                return Double.POSITIVE_INFINITY;
+            }
+            // Special case of cancellation: x^(a-1) (1-x)^(b-1) / B(a, b)
+            if (beta == 1) {
+                return -z;
             }
             return Double.NEGATIVE_INFINITY;
         } else {

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/BinomialDistribution.java

@@ -41,11 +41,7 @@ public BinomialDistribution(int trials,
             throw new DistributionException(DistributionException.NEGATIVE,
                                             trials);
         }
-        if (p < 0 ||
-            p > 1) {
-            throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
-        }
-
+        ArgumentUtils.checkProbability(p);
         probabilityOfSuccess = p;
         numberOfTrials = trials;
     }

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/CauchyDistribution.java

@@ -104,15 +104,13 @@ private static double cdf(double x) {
      */
     @Override
     public double inverseCumulativeProbability(double p) {
-        if (p < 0 ||
-            p > 1) {
-            throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
-        } else if (p == 0) {
+        ArgumentUtils.checkProbability(p);
+        if (p == 0) {
             return Double.NEGATIVE_INFINITY;
         } else  if (p == 1) {
             return Double.POSITIVE_INFINITY;
         }
-        return location + scale * Math.tan(Math.PI * (p - .5));
+        return location + scale * Math.tan(Math.PI * (p - 0.5));
     }
 
     /**

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/ChiSquaredDistribution.java

@@ -44,13 +44,29 @@ public double getDegreesOfFreedom() {
         return gamma.getShape() * 2;
     }
 
-    /** {@inheritDoc} */
+    /** {@inheritDoc}
+     *
+     * <p>Returns the limit when {@code x = 0}:
+     * <ul>
+     * <li>{@code df < 2}: Infinity
+     * <li>{@code df == 2}: 1 / 2
+     * <li>{@code df > 2}: 0
+     * </ul>
+     */
     @Override
     public double density(double x) {
         return gamma.density(x);
     }
 
-    /** {@inheritDoc} **/
+    /** {@inheritDoc}
+     *
+     * <p>Returns the limit when {@code x = 0}:
+     * <ul>
+     * <li>{@code df < 2}: Infinity
+     * <li>{@code df == 2}: log(1 / 2)
+     * <li>{@code df > 2}: -Infinity
+     * </ul>
+     */
     @Override
     public double logDensity(double x) {
         return gamma.logDensity(x);
@@ -68,6 +84,12 @@ public double survivalProbability(double x) {
         return gamma.survivalProbability(x);
     }
 
+    /** {@inheritDoc} */
+    @Override
+    public double inverseCumulativeProbability(double p) {
+        return gamma.inverseCumulativeProbability(p);
+    }
+
     /**
      * {@inheritDoc}
      *

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/DistributionException.java

@@ -40,6 +40,8 @@ class DistributionException extends IllegalArgumentException {
     static final String NEGATIVE = "Number %s is negative";
     /** Error message for "not strictly positive" condition when "x <= 0". */
     static final String NOT_STRICTLY_POSITIVE = "Number %s is not greater than 0";
+    /** Error message for "not strictly positive" condition when "x <= 0". */
+    static final String NOT_STRICTLY_POSITIVE_FINITE = "Number %s is not greater than 0 and finite";
 
     /** Serializable version identifier. */
     private static final long serialVersionUID = 20180119L;

commons-statistics-distribution/src/main/java/org/apache/commons/statistics/distribution/ExponentialDistribution.java

@@ -104,10 +104,8 @@ public double survivalProbability(double x)  {
      */
     @Override
     public double inverseCumulativeProbability(double p) {
-        if (p < 0 ||
-            p > 1) {
-            throw new DistributionException(DistributionException.INVALID_PROBABILITY, p);
-        } else if (p == 1) {
+        ArgumentUtils.checkProbability(p);
+        if (p == 1) {
             return Double.POSITIVE_INFINITY;
         }
         return -mean * Math.log1p(-p);