commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From luc <...@spaceroots.org>
Subject Re: [math] Attempt to circumvent some errors which seem to be platform-dependent.
Date Mon, 04 May 2015 14:20:04 GMT
Le 2015-05-04 14:32, Benedikt Ritter a écrit :
> Hello Luc,
> 
> 2015-05-04 13:43 GMT+02:00 <luc@apache.org>:
> 
>> Repository: commons-math
>> Updated Branches:
>>   refs/heads/master c8cb75243 -> c771c0080
>> 
>> 
>> Attempt to circumvent some errors which seem to be platform-dependent.
>> 
>> The Jenkins build often fails on code that seems to be perfectly
>> correct. Failures also do no always happen so they may depend on
>> platform. There were similar problems a few months ago that were
>> probably related to JIT bugs.
>> 
>> This fix simply tries to do the same thing as before, but with an
>> earlier detection of NaN in one case, and by comparing directly the 
>> bits
>> representation in another case, to avoid wrong optimizations.
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/c771c008
>> Tree: 
>> http://git-wip-us.apache.org/repos/asf/commons-math/tree/c771c008
>> Diff: 
>> http://git-wip-us.apache.org/repos/asf/commons-math/diff/c771c008
>> 
>> Branch: refs/heads/master
>> Commit: c771c0080b08abd80418c4e88f1be3efec828f0a
>> Parents: c8cb752
>> Author: Luc Maisonobe <luc@apache.org>
>> Authored: Mon May 4 13:43:27 2015 +0200
>> Committer: Luc Maisonobe <luc@apache.org>
>> Committed: Mon May 4 13:43:27 2015 +0200
>> 
>> ----------------------------------------------------------------------
>>  .../org/apache/commons/math4/util/FastMath.java | 28 
>> +++++++++-----------
>>  .../apache/commons/math4/util/FastMathTest.java |  4 +--
>>  2 files changed, 15 insertions(+), 17 deletions(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/c771c008/src/main/java/org/apache/commons/math4/util/FastMath.java
>> ----------------------------------------------------------------------
>> diff --git a/src/main/java/org/apache/commons/math4/util/FastMath.java
>> b/src/main/java/org/apache/commons/math4/util/FastMath.java
>> index 24bb857..fcd03ea 100644
>> --- a/src/main/java/org/apache/commons/math4/util/FastMath.java
>> +++ b/src/main/java/org/apache/commons/math4/util/FastMath.java
>> @@ -315,6 +315,9 @@ public class FastMath {
>>      /** Mask used to clear the non-sign part of a long. */
>>      private static final long MASK_NON_SIGN_LONG = 
>> 0x7fffffffffffffffl;
>> 
>> +    /** Bits representation of +1.0. */
>> +    private static final long PLUS_ONE_BITS = 0x3ff0000000000000L;
>> +
>>      /** 2^52 - double numbers this large must be integral (no 
>> fraction)
>> or NaN or Infinite */
>>      private static final double TWO_POWER_52 = 4503599627370496.0;
>>      /** 2^53 - double numbers this large must be even. */
>> @@ -1468,6 +1471,10 @@ public class FastMath {
>>              return x;
>>          }
>> 
>> +        if (y != y) { // Y is NaN
>> 
> 
> It really took me some time to understand this change. How about using
> Double.isNaN(double) instead? It does the same as the current code, but
> reads better, IMHO.

I agree but in this huge class this is how all NaNs are detected and 
there
are a bunch of such tests. I don't know the reason these existing tests
were done this way and not using Double.isNaN, it may well be 
performance related.
So for this class (and this class only), I prefer to do it the same way 
it
is already done a few lines above or below.

best regards,
Luc


> 
> Best regards,
> Benedikt
> 
> 
>> +            return y;
>> +        }
>> +
>>          if (x == 0) {
>>              long bits = Double.doubleToRawLongBits(x);
>>              if ((bits & 0x8000000000000000L) != 0) {
>> @@ -1485,18 +1492,13 @@ public class FastMath {
>> 
>>              if (y < 0) {
>>                  return Double.POSITIVE_INFINITY;
>> -            }
>> -            if (y > 0) {
>> +            } else {
>>                  return 0.0;
>>              }
>> 
>> -            return Double.NaN;
>>          }
>> 
>>          if (x == Double.POSITIVE_INFINITY) {
>> -            if (y != y) { // y is NaN
>> -                return y;
>> -            }
>>              if (y < 0.0) {
>>                  return 0.0;
>>              } else {
>> @@ -1505,21 +1507,17 @@ public class FastMath {
>>          }
>> 
>>          if (y == Double.POSITIVE_INFINITY) {
>> -            if (x * x == 1.0) {
>> -                return Double.NaN;
>> -            }
>> -
>> -            if (x * x > 1.0) {
>> +            long bitsAbsX = MASK_NON_SIGN_LONG &
>> Double.doubleToRawLongBits(x);
>> +            if (bitsAbsX > PLUS_ONE_BITS) {
>>                  return Double.POSITIVE_INFINITY;
>> -            } else {
>> +            } else if (bitsAbsX < PLUS_ONE_BITS) {
>>                  return 0.0;
>> +            } else {
>> +                return Double.NaN;
>>              }
>>          }
>> 
>>          if (x == Double.NEGATIVE_INFINITY) {
>> -            if (y != y) { // y is NaN
>> -                return y;
>> -            }
>> 
>>              if (y < 0) {
>>                  long yi = (long) y;
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/c771c008/src/test/java/org/apache/commons/math4/util/FastMathTest.java
>> ----------------------------------------------------------------------
>> diff --git 
>> a/src/test/java/org/apache/commons/math4/util/FastMathTest.java
>> b/src/test/java/org/apache/commons/math4/util/FastMathTest.java
>> index 5d36fea..06a1d07 100644
>> --- a/src/test/java/org/apache/commons/math4/util/FastMathTest.java
>> +++ b/src/test/java/org/apache/commons/math4/util/FastMathTest.java
>> @@ -29,8 +29,6 @@ import
>> org.apache.commons.math4.exception.MathArithmeticException;
>>  import org.apache.commons.math4.random.MersenneTwister;
>>  import org.apache.commons.math4.random.RandomGenerator;
>>  import org.apache.commons.math4.random.Well1024a;
>> -import org.apache.commons.math4.util.FastMath;
>> -import org.apache.commons.math4.util.Precision;
>>  import org.junit.Assert;
>>  import org.junit.Before;
>>  import org.junit.Ignore;
>> @@ -393,6 +391,8 @@ public class FastMathTest {
>> 
>>          Assert.assertTrue("pow(-2.0, 3.5) should be NaN",
>> Double.isNaN(FastMath.pow(-2.0, 3.5)));
>> 
>> +        Assert.assertTrue("pow(-0.0, NaN) should be NaN",
>> Double.isNaN(FastMath.pow(-0.0, Double.NaN)));
>> +
>>          // Added tests for a 100% coverage
>> 
>>          Assert.assertTrue("pow(+Inf, NaN) should be NaN",
>> Double.isNaN(FastMath.pow(Double.POSITIVE_INFINITY, Double.NaN)));
>> 
>> 

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


Mime
View raw message