You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by er...@apache.org on 2010/09/08 14:51:38 UTC

svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Author: erans
Date: Wed Sep  8 12:51:38 2010
New Revision: 995035

URL: http://svn.apache.org/viewvc?rev=995035&view=rev
Log:
Removed strict equality comparison.

Modified:
    commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java?rev=995035&r1=995034&r2=995035&view=diff
==============================================================================
--- commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java (original)
+++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java Wed Sep  8 12:51:38 2010
@@ -17,6 +17,7 @@
 package org.apache.commons.math.optimization.univariate;
 
 import org.apache.commons.math.FunctionEvaluationException;
+import org.apache.commons.math.util.MathUtils;
 import org.apache.commons.math.util.FastMath;
 import org.apache.commons.math.exception.NumberIsTooSmallException;
 import org.apache.commons.math.exception.NotStrictlyPositiveException;
@@ -219,12 +220,15 @@ public class BrentOptimizer extends Abst
                     } else {
                         b = u;
                     }
-                    if (fu <= fw || w == x) {
+                    if (fu <= fw ||
+                        MathUtils.equals(w, x)) {
                         v = w;
                         fv = fw;
                         w = u;
                         fw = fu;
-                    } else if (fu <= fv || v == x || v == w) {
+                    } else if (fu <= fv ||
+                               MathUtils.equals(v, x) ||
+                               MathUtils.equals(v, w)) {
                         v = u;
                         fv = fu;
                     }



Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by sebb <se...@gmail.com>.
On 8 September 2010 15:49, Luc Maisonobe <Lu...@free.fr> wrote:
> Le 08/09/2010 14:51, erans@apache.org a écrit :
>> Author: erans
>> Date: Wed Sep  8 12:51:38 2010
>> New Revision: 995035
>>
>> URL: http://svn.apache.org/viewvc?rev=995035&view=rev
>> Log:
>> Removed strict equality comparison.
>
> In some rare cases, strict equality comparison is desired (I don't know
> if it is the case here). In these cases, findbugs can be configured to
> ignore these cases, see the findbugs-exclude-filter.xml file for an example.

Also, it would be helpful to add a comment to the code to identify any
such cases.

> Luc
>
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java?rev=995035&r1=995034&r2=995035&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java Wed Sep  8 12:51:38 2010
>> @@ -17,6 +17,7 @@
>>  package org.apache.commons.math.optimization.univariate;
>>
>>  import org.apache.commons.math.FunctionEvaluationException;
>> +import org.apache.commons.math.util.MathUtils;
>>  import org.apache.commons.math.util.FastMath;
>>  import org.apache.commons.math.exception.NumberIsTooSmallException;
>>  import org.apache.commons.math.exception.NotStrictlyPositiveException;
>> @@ -219,12 +220,15 @@ public class BrentOptimizer extends Abst
>>                      } else {
>>                          b = u;
>>                      }
>> -                    if (fu <= fw || w == x) {
>> +                    if (fu <= fw ||
>> +                        MathUtils.equals(w, x)) {
>>                          v = w;
>>                          fv = fw;
>>                          w = u;
>>                          fw = fu;
>> -                    } else if (fu <= fv || v == x || v == w) {
>> +                    } else if (fu <= fv ||
>> +                               MathUtils.equals(v, x) ||
>> +                               MathUtils.equals(v, w)) {
>>                          v = u;
>>                          fv = fu;
>>                      }
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> 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


Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 10/09/2010 22:55, Gilles Sadowski a écrit :
>>> I don't know whether it is one of those rare cases. The algorithm
>>> description in ALGOL uses the "=" sign while the FORTRAN implementation (in
>>> the appendix) works around the problem by inversing the checks:
>>
>> It may be the case. There is a similar one in the Brent solver, the
>> semantic of the test is to check if we have just set one intermediate
>> value exactly to one of the bound or if it is a result of some
>> interpolation.
>>
>> Exact equality or inequality should be considered with the same care. In
>> many cases, they should be replaced by proximity checks. Proximity
>> checks need a threshold and it is very difficult to find a general one
>> (some people may well use numbers in the range of 1.0e-50 which is
>> perfectly computable in floating point).
>>
>> There is also one very special trick with doubles equality and
>> inequality: tests involving NaN always return false. So if for example x
>> is a NaN and y is a regular number (normal, subnormal or infinite), all
>> the following tests will return false, even the two last ones!
>>
>>  x < y, x > y, x <= y, x >= y, x == y, x != y, x == x
>>
>> This implies that replacing (x == y) by !(x != y) is a safe replacement
>> only if there are no NaNs involved.
>>
>> However, I don't think the test here has been written with NaNs in mind,
>> so I guess it really is a check for interpolation or no interpolation. I
>> did not check the algorithm, though.
> 
> I don't think so either.
> With the current change, the meaning is as close to "==" as possible since
> only two adjacent floating point numbers are considered equal by method
> "equals" from "MathUtils". So if checkstyle is happy with that, we could
> leave so without much risk, I think.

That's fine to me.

Luc

> 
> Gilles
> 
>>>>> -                    } else if (fu <= fv || v == x || v == w) {
>>>>> +                    } else if (fu <= fv ||
>>>>> +                               MathUtils.equals(v, x) ||
>>>>> +                               MathUtils.equals(v, w)) {
>>>>>                          v = u;
>>>>>                          fv = fu;
> 
> ---------------------------------------------------------------------
> 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


Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > I don't know whether it is one of those rare cases. The algorithm
> > description in ALGOL uses the "=" sign while the FORTRAN implementation (in
> > the appendix) works around the problem by inversing the checks:
> 
> It may be the case. There is a similar one in the Brent solver, the
> semantic of the test is to check if we have just set one intermediate
> value exactly to one of the bound or if it is a result of some
> interpolation.
> 
> Exact equality or inequality should be considered with the same care. In
> many cases, they should be replaced by proximity checks. Proximity
> checks need a threshold and it is very difficult to find a general one
> (some people may well use numbers in the range of 1.0e-50 which is
> perfectly computable in floating point).
> 
> There is also one very special trick with doubles equality and
> inequality: tests involving NaN always return false. So if for example x
> is a NaN and y is a regular number (normal, subnormal or infinite), all
> the following tests will return false, even the two last ones!
> 
>  x < y, x > y, x <= y, x >= y, x == y, x != y, x == x
> 
> This implies that replacing (x == y) by !(x != y) is a safe replacement
> only if there are no NaNs involved.
> 
> However, I don't think the test here has been written with NaNs in mind,
> so I guess it really is a check for interpolation or no interpolation. I
> did not check the algorithm, though.

I don't think so either.
With the current change, the meaning is as close to "==" as possible since
only two adjacent floating point numbers are considered equal by method
"equals" from "MathUtils". So if checkstyle is happy with that, we could
leave so without much risk, I think.

Gilles

> >>> -                    } else if (fu <= fv || v == x || v == w) {
> >>> +                    } else if (fu <= fv ||
> >>> +                               MathUtils.equals(v, x) ||
> >>> +                               MathUtils.equals(v, w)) {
> >>>                          v = u;
> >>>                          fv = fu;

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


Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 08/09/2010 17:37, Gilles Sadowski a écrit :
>>> URL: http://svn.apache.org/viewvc?rev=995035&view=rev
>>> Log:
>>> Removed strict equality comparison.
>>
>> In some rare cases, strict equality comparison is desired (I don't know
>> if it is the case here). In these cases, findbugs can be configured to
>> ignore these cases, see the findbugs-exclude-filter.xml file for an example.
> 
> I don't know whether it is one of those rare cases. The algorithm
> description in ALGOL uses the "=" sign while the FORTRAN implementation (in
> the appendix) works around the problem by inversing the checks:

It may be the case. There is a similar one in the Brent solver, the
semantic of the test is to check if we have just set one intermediate
value exactly to one of the bound or if it is a result of some
interpolation.

Exact equality or inequality should be considered with the same care. In
many cases, they should be replaced by proximity checks. Proximity
checks need a threshold and it is very difficult to find a general one
(some people may well use numbers in the range of 1.0e-50 which is
perfectly computable in floating point).

There is also one very special trick with doubles equality and
inequality: tests involving NaN always return false. So if for example x
is a NaN and y is a regular number (normal, subnormal or infinite), all
the following tests will return false, even the two last ones!

 x < y, x > y, x <= y, x >= y, x == y, x != y, x == x

This implies that replacing (x == y) by !(x != y) is a safe replacement
only if there are no NaNs involved.

However, I don't think the test here has been written with NaNs in mind,
so I guess it really is a check for interpolation or no interpolation. I
did not check the algorithm, though.

Luc

> 170 IF ((FU.GT.FW).AND.(W.NE.X)) GO TO 180
>     V = W
>     FV = FW
>     W = U
>     FW = FU
>     GO TO 10
> 180 etc.
> 
>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java?rev=995035&r1=995034&r2=995035&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java (original)
>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java Wed Sep  8 12:51:38 2010
>>> @@ -17,6 +17,7 @@
>>>  package org.apache.commons.math.optimization.univariate;
>>>  
>>>  import org.apache.commons.math.FunctionEvaluationException;
>>> +import org.apache.commons.math.util.MathUtils;
>>>  import org.apache.commons.math.util.FastMath;
>>>  import org.apache.commons.math.exception.NumberIsTooSmallException;
>>>  import org.apache.commons.math.exception.NotStrictlyPositiveException;
>>> @@ -219,12 +220,15 @@ public class BrentOptimizer extends Abst
>>>                      } else {
>>>                          b = u;
>>>                      }
>>> -                    if (fu <= fw || w == x) {
>>> +                    if (fu <= fw ||
>>> +                        MathUtils.equals(w, x)) {
>>>                          v = w;
>>>                          fv = fw;
>>>                          w = u;
>>>                          fw = fu;
>>> -                    } else if (fu <= fv || v == x || v == w) {
>>> +                    } else if (fu <= fv ||
>>> +                               MathUtils.equals(v, x) ||
>>> +                               MathUtils.equals(v, w)) {
>>>                          v = u;
>>>                          fv = fu;
>>>                      }
> 
> ---------------------------------------------------------------------
> 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


Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> > URL: http://svn.apache.org/viewvc?rev=995035&view=rev
> > Log:
> > Removed strict equality comparison.
> 
> In some rare cases, strict equality comparison is desired (I don't know
> if it is the case here). In these cases, findbugs can be configured to
> ignore these cases, see the findbugs-exclude-filter.xml file for an example.

I don't know whether it is one of those rare cases. The algorithm
description in ALGOL uses the "=" sign while the FORTRAN implementation (in
the appendix) works around the problem by inversing the checks:
170 IF ((FU.GT.FW).AND.(W.NE.X)) GO TO 180
    V = W
    FV = FW
    W = U
    FW = FU
    GO TO 10
180 etc.

> > Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
> > URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java?rev=995035&r1=995034&r2=995035&view=diff
> > ==============================================================================
> > --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java (original)
> > +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java Wed Sep  8 12:51:38 2010
> > @@ -17,6 +17,7 @@
> >  package org.apache.commons.math.optimization.univariate;
> >  
> >  import org.apache.commons.math.FunctionEvaluationException;
> > +import org.apache.commons.math.util.MathUtils;
> >  import org.apache.commons.math.util.FastMath;
> >  import org.apache.commons.math.exception.NumberIsTooSmallException;
> >  import org.apache.commons.math.exception.NotStrictlyPositiveException;
> > @@ -219,12 +220,15 @@ public class BrentOptimizer extends Abst
> >                      } else {
> >                          b = u;
> >                      }
> > -                    if (fu <= fw || w == x) {
> > +                    if (fu <= fw ||
> > +                        MathUtils.equals(w, x)) {
> >                          v = w;
> >                          fv = fw;
> >                          w = u;
> >                          fw = fu;
> > -                    } else if (fu <= fv || v == x || v == w) {
> > +                    } else if (fu <= fv ||
> > +                               MathUtils.equals(v, x) ||
> > +                               MathUtils.equals(v, w)) {
> >                          v = u;
> >                          fv = fu;
> >                      }

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


Re: svn commit: r995035 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 08/09/2010 14:51, erans@apache.org a écrit :
> Author: erans
> Date: Wed Sep  8 12:51:38 2010
> New Revision: 995035
> 
> URL: http://svn.apache.org/viewvc?rev=995035&view=rev
> Log:
> Removed strict equality comparison.

In some rare cases, strict equality comparison is desired (I don't know
if it is the case here). In these cases, findbugs can be configured to
ignore these cases, see the findbugs-exclude-filter.xml file for an example.

Luc

> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java?rev=995035&r1=995034&r2=995035&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math/optimization/univariate/BrentOptimizer.java Wed Sep  8 12:51:38 2010
> @@ -17,6 +17,7 @@
>  package org.apache.commons.math.optimization.univariate;
>  
>  import org.apache.commons.math.FunctionEvaluationException;
> +import org.apache.commons.math.util.MathUtils;
>  import org.apache.commons.math.util.FastMath;
>  import org.apache.commons.math.exception.NumberIsTooSmallException;
>  import org.apache.commons.math.exception.NotStrictlyPositiveException;
> @@ -219,12 +220,15 @@ public class BrentOptimizer extends Abst
>                      } else {
>                          b = u;
>                      }
> -                    if (fu <= fw || w == x) {
> +                    if (fu <= fw ||
> +                        MathUtils.equals(w, x)) {
>                          v = w;
>                          fv = fw;
>                          w = u;
>                          fw = fu;
> -                    } else if (fu <= fv || v == x || v == w) {
> +                    } else if (fu <= fv ||
> +                               MathUtils.equals(v, x) ||
> +                               MathUtils.equals(v, w)) {
>                          v = u;
>                          fv = fu;
>                      }
> 
> 
> 


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