From dev-return-115236-apmail-commons-dev-archive=commons.apache.org@commons.apache.org Tue Jul 07 12:56:13 2009
Return-Path:
Delivered-To: apmail-commons-dev-archive@www.apache.org
Received: (qmail 77799 invoked from network); 7 Jul 2009 12:56:12 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3)
by minotaur.apache.org with SMTP; 7 Jul 2009 12:56:12 -0000
Received: (qmail 41083 invoked by uid 500); 7 Jul 2009 12:56:22 -0000
Delivered-To: apmail-commons-dev-archive@commons.apache.org
Received: (qmail 40973 invoked by uid 500); 7 Jul 2009 12:56:22 -0000
Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
List-Help:
List-Unsubscribe:
List-Post:
List-Id:
Reply-To: "Commons Developers List"
Delivered-To: mailing list dev@commons.apache.org
Received: (qmail 40963 invoked by uid 99); 7 Jul 2009 12:56:22 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 07 Jul 2009 12:56:22 +0000
X-ASF-Spam-Status: No, hits=-0.0 required=10.0
tests=SPF_PASS
X-Spam-Check-By: apache.org
Received-SPF: pass (athena.apache.org: domain of phil.steitz@gmail.com designates 209.85.216.184 as permitted sender)
Received: from [209.85.216.184] (HELO mail-px0-f184.google.com) (209.85.216.184)
by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 07 Jul 2009 12:56:12 +0000
Received: by pxi14 with SMTP id 14so4166416pxi.10
for ; Tue, 07 Jul 2009 05:55:52 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=gmail.com; s=gamma;
h=domainkey-signature:received:received:message-id:date:from
:user-agent:mime-version:to:subject:references:in-reply-to
:content-type:content-transfer-encoding;
bh=LuLD/DUsM2jLvp+YS/nDpjbr7PKt5HkTEQgZPPQ6MHw=;
b=IQTpiJj/oJnWI+Re9vLlpy0qf6FBcZVEDFfpwQ/PTbuzLUG1EclC+NEB4f+1EgpSB1
Mpy5A3UzxxPx2PAqZhP87mYrtgBgA+5zZVmJkG1N87gZzeyjaZqRAjzsu/Tj9HknCKBi
O1AcHniq8pc4//hiGhyBJ6DHMKqJnVzKrF9wA=
DomainKey-Signature: a=rsa-sha1; c=nofws;
d=gmail.com; s=gamma;
h=message-id:date:from:user-agent:mime-version:to:subject:references
:in-reply-to:content-type:content-transfer-encoding;
b=bhaLHuQkB4lfS2jNr1xOrlTM8xNPA55wgJfMgz7Uahpk46xwiPagCiTyG8DLfvFONk
X9h7EJA81sFj12IMWODnzeFuDABIXN1ZxWWKwf2xi1ruWqgVAcvWP/cdZH76bus1rfrW
1LKl0H+CmFwr06FVb7EAYWWd7MHU1p66UbLKo=
Received: by 10.114.158.1 with SMTP id g1mr9657545wae.18.1246970966229;
Tue, 07 Jul 2009 05:49:26 -0700 (PDT)
Received: from phil-steitzs-macbook-pro.local (pool-96-227-146-177.phlapa.east.verizon.net [96.227.146.177])
by mx.google.com with ESMTPS id m31sm13156908wag.31.2009.07.07.05.49.23
(version=SSLv3 cipher=RC4-MD5);
Tue, 07 Jul 2009 05:49:25 -0700 (PDT)
Message-ID: <4A534452.9060305@gmail.com>
Date: Tue, 07 Jul 2009 08:49:22 -0400
From: Phil Steitz
User-Agent: Thunderbird 2.0.0.22 (Macintosh/20090605)
MIME-Version: 1.0
To: Commons Developers List
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/
References: <558622690.6375651246959888653.JavaMail.root@spooler6-g27.priv.proxad.net>
In-Reply-To: <558622690.6375651246959888653.JavaMail.root@spooler6-g27.priv.proxad.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-Virus-Checked: Checked by ClamAV on apache.org
luc.maisonobe@free.fr wrote:
> 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 ...
>
Agree with your analysis. I was....slowly...coming to the same
conclusion ;)
> 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).
Looks like only Brent was fixed in MATH-204, but others are OK, except
for Secant, which we should probably fix.
> This behaviour was used in a test case (testBracketCornerSolution). I have removed this test but feel uncomfortable with this choice.
>
The contract of the method was changed - and documented - so removing
this test case is OK. Probably should replace it with a test confirming
success when a root is encountered.
> Could someone either acknowledge this choice or propose another fix for the problem ?
>
I am +1 on the changes. Since the release notes will be generated from
changes.xml, it is probably best to add some more commentary there on
the contract change for UnivariateSolverUtils.bracket. Since solvers
are pluggable in the inverse probability distribution computations, it
is probably also a good idea to make the following change to the
UnivariateRealSolver.solve javadoc:
s/A solver may require that the interval brackets a single zero root./A
solver may require that the interval brackets a single zero root.
Solvers that do require bracketing should be able to handle the case
where one of the endpoints is itself a root./
Phil
> 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
>>
>> * -
lowerBound <= a < initial < b <= upperBound
>>
>> - * -
f(a) * f(b) < 0
>> + * -
f(a) * f(b) <= 0
>> *
>> * If f is continuous on [a,b],
this means that
>> a
>> * and b
bracket a root of f.
>> @@ -141,7 +141,7 @@
>> * function at a
and b
and keeps
>> moving
>> * the endpoints out by one unit each time through a loop that
>> terminates
>> * when one of the following happens:
>> - * -
f(a) * f(b) < 0
-- success!
>> + * -
f(a) * f(b) <= 0
-- success!
>> * -
a = lower
and b = upper
>> * -- ConvergenceException
>> * -
maximumIterations
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 @@
>>
>>
>>
>> +
>> + Fixed a bracketing issue in inverse cumulative probability
>> computation
>> +
>>
>> Added a check for too few rows with respect to the number of
>> predictors in linear regression
>>
>>
>> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org