You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ah...@apache.org on 2019/12/09 15:48:08 UTC

[commons-numbers] branch master updated (0887c25 -> 6a7ce94)

This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git.


    from 0887c25  sqrt(): Drop copySign(0, imaginary) when imaginary is already zero.
     new 087d40a  Clean up sqrt().
     new 1d63ae2  Ensure sqrt() always functions on finite input.
     new 6a7ce94  PMD: Ignore field real and method real() match in Complex.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/commons/numbers/complex/Complex.java    | 56 +++++++++++++++-------
 .../commons/numbers/complex/CReferenceTest.java    | 36 ++++++++++----
 src/main/resources/pmd/pmd-ruleset.xml             |  7 +++
 3 files changed, 71 insertions(+), 28 deletions(-)


[commons-numbers] 03/03: PMD: Ignore field real and method real() match in Complex.

Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit 6a7ce94ac7b4a3e1c612c2e7e05b3f750c9441db
Author: aherbert <ah...@apache.org>
AuthorDate: Mon Dec 9 15:46:58 2019 +0000

    PMD: Ignore field real and method real() match in Complex.
---
 src/main/resources/pmd/pmd-ruleset.xml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/main/resources/pmd/pmd-ruleset.xml b/src/main/resources/pmd/pmd-ruleset.xml
index b48a3b4..2b800bb 100644
--- a/src/main/resources/pmd/pmd-ruleset.xml
+++ b/src/main/resources/pmd/pmd-ruleset.xml
@@ -124,4 +124,11 @@
     </properties>
   </rule>
 
+  <rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingMethodName">
+    <properties>
+      <property name="violationSuppressXPath"
+        value="//ClassOrInterfaceDeclaration[@Image='Complex']"/>
+    </properties>
+  </rule>
+
 </ruleset>


[commons-numbers] 01/03: Clean up sqrt().

Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit 087d40af801116902d19aeb12804552b804f0ffd
Author: aherbert <ah...@apache.org>
AuthorDate: Mon Dec 9 13:01:09 2019 +0000

    Clean up sqrt().
    
    Removed the commented out code to compute the sqrt() in polar
    coordinates.
    
    Add note about computing for real only complex numbers to the
    CReferenceTest.
---
 .../org/apache/commons/numbers/complex/Complex.java   |  7 -------
 .../commons/numbers/complex/CReferenceTest.java       | 19 ++++++++++---------
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java b/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
index d4ddf26..8ce7574 100644
--- a/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
+++ b/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
@@ -1690,13 +1690,6 @@ public final class Complex implements Serializable  {
      * @return the square root of the complex number.
      */
     private static Complex sqrt(double real, double imaginary) {
-//        final double abs = getAbsolute(real, imaginary);
-//        final double sqrtAbs = Math.sqrt(abs);
-//        final double halfArg = getArgument(real, imaginary) / 2;
-//        final double re = sqrtAbs * Math.cos(halfArg);
-//        final double im = sqrtAbs * Math.sin(halfArg);
-//        return new Complex(re, im);
-
         // Special case for infinite imaginary for all real including nan
         if (Double.isInfinite(imaginary)) {
             return new Complex(Double.POSITIVE_INFINITY, imaginary);
diff --git a/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java b/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
index 22b8cf2..2e817b7 100644
--- a/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
+++ b/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
@@ -399,24 +399,25 @@ public class CReferenceTest {
 
     @Test
     public void testSqrt() {
-        // TODO: GNU g++ computes in polar coordinates.
-        // Test other reference implementations.
-        // This gives strange results for real negative and imaginary 0.
+        // Note: When computed in polar coordinates:
+        //   real = (x^2 + y^2)^0.25 * cos(0.5 * atan2(y, x))
+        // If x is positive and y is +/-0.0 atan2 returns +/-0.
+        // If x is negative and y is +/-0.0 atan2 returns +/-PI.
+        // This causes problems as
+        //   cos(0.5 * PI) = 6.123233995736766e-17
+        // assert: Math.cos(Math.acos(0)) != 0.0
+        // Thus polar computation will produce incorrect output when
+        // there is no imaginary component and real is negative.
+        // The computation should be done for real only numbers separately.
 
-        // Polar result
-        //assertComplex(-2, 0.0, Complex::sqrt, 8.6595605623549341e-17, 1.4142135623730951);
         assertComplex(-2, 0.0, Complex::sqrt, 0, 1.4142135623730951);
         assertComplex(-2, 0.5, Complex::sqrt, 0.17543205637629397, 1.425053124063947, 5);
         assertComplex(-2, 1, Complex::sqrt, 0.3435607497225126, 1.4553466902253549, 3);
         assertComplex(-2, 2, Complex::sqrt, 0.64359425290558281, 1.5537739740300374, 2);
-        // Polar result
-        //assertComplex(-1, 0.0, Complex::sqrt, 6.123233995736766e-17, 1);
         assertComplex(-1, 0.0, Complex::sqrt, 0, 1);
         assertComplex(-1, 0.5, Complex::sqrt, 0.24293413587832291, 1.0290855136357462, 3);
         assertComplex(-1, 1, Complex::sqrt, 0.45508986056222739, 1.0986841134678098);
         assertComplex(-1, 2, Complex::sqrt, 0.78615137775742339, 1.2720196495140688);
-        // Polar result
-        //assertComplex(-0.5, 0.0, Complex::sqrt, 4.329780281177467e-17, 0.70710678118654757);
         assertComplex(-0.5, 0.0, Complex::sqrt, 0, 0.70710678118654757);
         assertComplex(-0.5, 0.5, Complex::sqrt, 0.3217971264527914, 0.77688698701501868, 2);
         assertComplex(-0.5, 1, Complex::sqrt, 0.55589297025142126, 0.89945371997393353);


[commons-numbers] 02/03: Ensure sqrt() always functions on finite input.

Posted by ah...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-numbers.git

commit 1d63ae2bd28ccce68832f6348ad37961504908a6
Author: aherbert <ah...@apache.org>
AuthorDate: Mon Dec 9 15:46:34 2019 +0000

    Ensure sqrt() always functions on finite input.
    
    Any finite complex can be square rooted. If computation of the new
    magnitude overflows then the computation can be scaled down, computed
    then scaled back. This ensures infinity is never returned for a finite
    complex.
---
 .../apache/commons/numbers/complex/Complex.java    | 49 +++++++++++++++++-----
 .../commons/numbers/complex/CReferenceTest.java    | 17 +++++++-
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java b/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
index 8ce7574..8e57ac0 100644
--- a/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
+++ b/commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java
@@ -1704,19 +1704,33 @@ public final class Complex implements Serializable  {
                     }
                     return new Complex(sqrtAbs, imaginary);
                 }
-                final double abs = getAbsolute(real, imaginary);
-                final double av = (Math.abs(real) + abs) / 2;
-                if (av == Double.POSITIVE_INFINITY) {
-                    // Compute in polar coords.
-                    // This handles extreme values that fail in the cartesian representation.
-                    final double sqrtAbs = Math.sqrt(abs);
-                    final double halfArg = getArgument(real, imaginary) / 2;
-                    final double re = sqrtAbs * Math.cos(halfArg);
-                    final double im = sqrtAbs * Math.sin(halfArg);
-                    return new Complex(re, im);
+                // Get the absolute of the real
+                double absA = Math.abs(real);
+                // Compute |a + b i|
+                double absC = getAbsolute(real, imaginary);
+
+                // t = sqrt((|a| + |a + b i|) / 2)
+                // This is always representable as this complex is finite.
+                double t;
+
+                // Overflow safe
+                if (absC == Double.POSITIVE_INFINITY) {
+                    // Complex is too large.
+                    // Divide by the largest absolute component,
+                    // compute the required sqrt and then scale back.
+                    // Use the equality: sqrt(n) = sqrt(s) * sqrt(n/s)
+                    // t = sqrt(max) * sqrt((|a|/max + |a + b i|/max) / 2)
+                    // Note: The function may be non-monotonic at the junction.
+                    // The alternative of returning infinity for a finite input is worse.
+                    final double max = Math.max(absA, Math.abs(imaginary));
+                    absA /= max;
+                    absC = getAbsolute(absA, imaginary / max);
+                    t = Math.sqrt(max) * Math.sqrt((absA + absC) / 2);
+                } else {
+                    // Over-flow safe average
+                    t = Math.sqrt(average(absA, absC));
                 }
 
-                final double t = Math.sqrt(av);
                 if (real >= 0) {
                     return new Complex(t, imaginary / (2 * t));
                 }
@@ -1740,6 +1754,19 @@ public final class Complex implements Serializable  {
     }
 
     /**
+     * Compute the average of two positive finite values in an overflow safe manner.
+     *
+     * @param a the first value
+     * @param b the second value
+     * @return the average
+     */
+    private static double average(double a, double b) {
+        return (a < b) ?
+            a + (b - a) / 2 :
+            b + (a - b) / 2;
+    }
+
+    /**
      * Compute the
      * <a href="http://mathworld.wolfram.com/Tangent.html">
      * tangent</a> of this complex number.
diff --git a/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java b/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
index 2e817b7..662a972 100644
--- a/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
+++ b/commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java
@@ -35,7 +35,6 @@ import java.util.function.UnaryOperator;
  */
 public class CReferenceTest {
     private static final double inf = Double.POSITIVE_INFINITY;
-    private static final double nan = Double.NaN;
 
     /**
      * Assert the two numbers are equal within the provided units of least precision.
@@ -401,6 +400,7 @@ public class CReferenceTest {
     public void testSqrt() {
         // Note: When computed in polar coordinates:
         //   real = (x^2 + y^2)^0.25 * cos(0.5 * atan2(y, x))
+        //   imag = (x^2 + y^2)^0.25 * sin(0.5 * atan2(y, x))
         // If x is positive and y is +/-0.0 atan2 returns +/-0.
         // If x is negative and y is +/-0.0 atan2 returns +/-PI.
         // This causes problems as
@@ -410,6 +410,21 @@ public class CReferenceTest {
         // there is no imaginary component and real is negative.
         // The computation should be done for real only numbers separately.
 
+        // Check overflow safe.
+        final double a = Double.MAX_VALUE;
+        final double b = a / 4;
+        Assertions.assertEquals(inf, Complex.ofCartesian(a, b).abs(), "Expected overflow");
+        // Compute the expected new magnitude by expressing b as a scale factor of a:
+        // (x^2 + y^2)^0.25
+        // = sqrt(sqrt(a^2 + (b/a)^2 * a^2))
+        // = sqrt(sqrt((1+(b/a)^2) * a^2))
+        // = sqrt(sqrt((1+(b/a)^2))) * sqrt(a)
+        final double newAbs = Math.sqrt(Math.sqrt(1 + (b / a) * (b / a))) * Math.sqrt(a);
+        assertComplex(a, b, Complex::sqrt, newAbs * Math.cos(0.5 * Math.atan2(b, a)),
+                                           newAbs * Math.sin(0.5 * Math.atan2(b, a)), 3);
+        assertComplex(b, a, Complex::sqrt, newAbs * Math.cos(0.5 * Math.atan2(a, b)),
+                                           newAbs * Math.sin(0.5 * Math.atan2(a, b)), 2);
+
         assertComplex(-2, 0.0, Complex::sqrt, 0, 1.4142135623730951);
         assertComplex(-2, 0.5, Complex::sqrt, 0.17543205637629397, 1.425053124063947, 5);
         assertComplex(-2, 1, Complex::sqrt, 0.3435607497225126, 1.4553466902253549, 3);