commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From luc.maison...@free.fr
Subject Re: svn commit: r791766 - in /commons/proper/math/trunk/src: java/org/apache/commons/math/analysis/solvers/ java/org/apache/commons/math/distribution/ site/xdoc/ test/org/apache/commons/math/analysis/solvers/ test/org/apache/commons/math/distribution/
Date Tue, 07 Jul 2009 09:44:48 GMT
The following commit solved MATH-280. Here are some explanations about the change.

The issue was triggerred by a bracketing attempt in AbstractContinuousDistribution.bracket.
This bracketing attempt starts from an initial value (here it was exactly 1.0 since it was
mean + standard deviation and we were working on a normalized gaussian). From this initial
value, it increases an interval by moving both a lower bound and an upper bound by steps of
1.0, so the intervals used are [0.0, 2.0], [-1.0, 3.0], [-2.0, 4.0] ... In fact, at the first
iteration the upper bound (2.0) is exactly the root of the function, so f(b)=0 and the loop
ends due to criteria f(a)*f(b) > 0. However the following check was f(a)*f(b) >= 0 and
an exception was triggered.

In fact, there was already a protection against exact solutions in the AbstractContinuousDistribution.inverseCumulativeProbability
method. The exception thrown by the bracket method was caught and the domain endpoint were
checked to see if they are a solution to the
 inverse cumulative computation problem. Here, is was not the case because the solution was
an intermediate point (2.0), and the endpoints were 0.0 and Double.MAX_VALUE. So the exception
was rethrown ...

The solution I used was to have consistent tests in the bracket method: both now use f(a)*f(b)
> 0 and the correct result is returned.

This however have changed the behaviour of the bracket method: it doesn't throw an exception
anymore when an exact root is encountered. This seemed a logical choice as there was already
some defensive code in the upper level methods and I think we have fixed the solvers one year
ago to find root exactly at endpoints (see MATH-204). This behaviour was used in a test case
(testBracketCornerSolution). I have removed this test but feel uncomfortable with this choice.

Could someone either acknowledge this choice or propose another fix for the problem ?

thanks,
Luc

----- luc@apache.org a écrit :

> Author: luc
> Date: Tue Jul  7 09:19:46 2009
> New Revision: 791766
> 
> URL: http://svn.apache.org/viewvc?rev=791766&view=rev
> Log:
> fixed a bracketing issue due to inconsistent checks
> JIRA: MATH-280
> 
> Modified:
>    
> commons/proper/math/trunk/src/java/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtils.java
>    
> commons/proper/math/trunk/src/java/org/apache/commons/math/distribution/AbstractContinuousDistribution.java
>     commons/proper/math/trunk/src/site/xdoc/changes.xml
>    
> commons/proper/math/trunk/src/test/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtilsTest.java
>    
> commons/proper/math/trunk/src/test/org/apache/commons/math/distribution/NormalDistributionTest.java
> 
> Modified:
> commons/proper/math/trunk/src/java/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtils.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtils.java?rev=791766&r1=791765&r2=791766&view=diff
> ==============================================================================
> ---
> commons/proper/math/trunk/src/java/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtils.java
> (original)
> +++
> commons/proper/math/trunk/src/java/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtils.java
> Tue Jul  7 09:19:46 2009
> @@ -131,7 +131,7 @@
>       /**
>       * This method attempts to find two values a and b satisfying
> <ul>
>       * <li> <code> lowerBound <= a < initial < b <= upperBound</code>
> </li>
> -     * <li> <code> f(a) * f(b) < 0 </code> </li>
> +     * <li> <code> f(a) * f(b) <= 0 </code> </li>
>       * </ul>
>       * If f is continuous on <code>[a,b],</code> this means that
> <code>a</code>
>       * and <code>b</code> bracket a root of f.
> @@ -141,7 +141,7 @@
>       * function at <code>a</code> and <code>b</code> and keeps
> moving
>       * the endpoints out by one unit each time through a loop that
> terminates 
>       * when one of the following happens: <ul>
> -     * <li> <code> f(a) * f(b) < 0 </code> --  success!</li>
> +     * <li> <code> f(a) * f(b) <= 0 </code> --  success!</li>
>       * <li> <code> a = lower </code> and <code> b = upper</code>

>       * -- ConvergenceException </li>
>       * <li> <code> maximumIterations</code> iterations elapse 
> @@ -195,7 +195,7 @@
>          } while ((fa * fb > 0.0) && (numIterations <
> maximumIterations) && 
>                  ((a > lowerBound) || (b < upperBound)));
>     
> -        if (fa * fb >= 0.0 ) {
> +        if (fa * fb > 0.0 ) {
>              throw new ConvergenceException(
>                        "number of iterations={0}, maximum
> iterations={1}, " +
>                        "initial={2}, lower bound={3}, upper bound={4},
> final a value={5}, " +
> 
> Modified:
> commons/proper/math/trunk/src/java/org/apache/commons/math/distribution/AbstractContinuousDistribution.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/distribution/AbstractContinuousDistribution.java?rev=791766&r1=791765&r2=791766&view=diff
> ==============================================================================
> ---
> commons/proper/math/trunk/src/java/org/apache/commons/math/distribution/AbstractContinuousDistribution.java
> (original)
> +++
> commons/proper/math/trunk/src/java/org/apache/commons/math/distribution/AbstractContinuousDistribution.java
> Tue Jul  7 09:19:46 2009
> @@ -65,7 +65,7 @@
>          }
>  
>          // by default, do simple root finding using bracketing and
> default solver.
> -        // subclasses can overide if there is a better method.
> +        // subclasses can override if there is a better method.
>          UnivariateRealFunction rootFindingFunction =
>              new UnivariateRealFunction() {
>              public double value(double x) throws
> FunctionEvaluationException {
> 
> Modified: commons/proper/math/trunk/src/site/xdoc/changes.xml
> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/site/xdoc/changes.xml?rev=791766&r1=791765&r2=791766&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/site/xdoc/changes.xml (original)
> +++ commons/proper/math/trunk/src/site/xdoc/changes.xml Tue Jul  7
> 09:19:46 2009
> @@ -39,6 +39,9 @@
>    </properties>
>    <body>
>      <release version="2.0" date="TBD" description="TBD">
> +      <action dev="luc" type="fix" issue="MATH-280">
> +        Fixed a bracketing issue in inverse cumulative probability
> computation
> +      </action>
>        <action dev="luc" type="add" issue="MATH-279" due-to="Michael
> Bjorkegren">
>          Added a check for too few rows with respect to the number of
> predictors in linear regression
>        </action>
> 
> Modified:
> commons/proper/math/trunk/src/test/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtilsTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtilsTest.java?rev=791766&r1=791765&r2=791766&view=diff
> ==============================================================================
> ---
> commons/proper/math/trunk/src/test/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtilsTest.java
> (original)
> +++
> commons/proper/math/trunk/src/test/org/apache/commons/math/analysis/solvers/UnivariateRealSolverUtilsTest.java
> Tue Jul  7 09:19:46 2009
> @@ -17,13 +17,12 @@
>  
>  package org.apache.commons.math.analysis.solvers;
>  
> -import org.apache.commons.math.ConvergenceException;
> +import junit.framework.TestCase;
> +
>  import org.apache.commons.math.MathException;
>  import org.apache.commons.math.analysis.SinFunction;
>  import org.apache.commons.math.analysis.UnivariateRealFunction;
>  
> -import junit.framework.TestCase;
> -
>  /**
>   * @version $Revision$ $Date$
>   */
> @@ -91,15 +90,6 @@
>          assertTrue(sin.value(result[1]) > 0);
>      }
>      
> -    public void testBracketCornerSolution() throws MathException {
> -        try {
> -            UnivariateRealSolverUtils.bracket(sin, 1.5, 0, 2.0); 
> -            fail("Expecting ConvergenceException");
> -        } catch (ConvergenceException ex) {
> -            // expected
> -        }
> -    }
> -    
>      public void testBadParameters() throws MathException {
>          try { // null function
>              UnivariateRealSolverUtils.bracket(null, 1.5, 0, 2.0);
> 
> Modified:
> commons/proper/math/trunk/src/test/org/apache/commons/math/distribution/NormalDistributionTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/org/apache/commons/math/distribution/NormalDistributionTest.java?rev=791766&r1=791765&r2=791766&view=diff
> ==============================================================================
> ---
> commons/proper/math/trunk/src/test/org/apache/commons/math/distribution/NormalDistributionTest.java
> (original)
> +++
> commons/proper/math/trunk/src/test/org/apache/commons/math/distribution/NormalDistributionTest.java
> Tue Jul  7 09:19:46 2009
> @@ -17,6 +17,8 @@
>  
>  package org.apache.commons.math.distribution;
>  
> +import org.apache.commons.math.MathException;
> +
>  /**
>   * Test cases for NormalDistribution.
>   * Extends ContinuousDistributionAbstractTest.  See class javadoc
> for
> @@ -161,4 +163,11 @@
>              }
>          } 
>     }
> +
> +    public void testMath280() throws MathException {
> +        NormalDistribution normal = new NormalDistributionImpl(0,1);
> +        double result =
> normal.inverseCumulativeProbability(0.9772498680518209);
> +        assertEquals(2.0, result, 1.0e-12);
> +    }
> +
>  }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message