Also, note that commit log messages are not published with the source code.
So please consider adding suitable comments in the source code itself. The commit log should contain sufficient detail to understand why the commit was done; the source code needs to contain the detail to understand the source on its own. On 4 May 2015 at 13:32, Benedikt Ritter <brit...@apache.org> wrote: > Hello Luc, > > 2015-05-04 13:43 GMT+02:00 <l...@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 <l...@apache.org> >> Authored: Mon May 4 13:43:27 2015 +0200 >> Committer: Luc Maisonobe <l...@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. > > 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))); >> >> > > > -- > http://people.apache.org/~britter/ > http://www.systemoutprintln.de/ > http://twitter.com/BenediktRitter > http://github.com/britter --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
