[
https://issues.apache.org/jira/browse/NUMBERS-188?focusedWorklogId=790889&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-790889
]
ASF GitHub Bot logged work on NUMBERS-188:
------------------------------------------
Author: ASF GitHub Bot
Created on: 14/Jul/22 11:39
Start Date: 14/Jul/22 11:39
Worklog Time Spent: 10m
Work Description: aherbert commented on code in PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#discussion_r920485833
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexConstructor.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+/**
+ * Define a constructor for a Complex.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexConstructor<R> {
+
+ /**
+ * Represents a function that accepts real and imaginary part of complex
number and returns an object.
+ * @param real real part
+ * @param imaginary imaginary part
+ * @return R
+ */
+ R apply(double real, double imaginary);
+
+ /**
+ * Represents a helper function to compose a unary operator.
+ * @param after ComplexUnaryOperator to be composed
+ * @return ComplexConstructor
+ */
+ default ComplexConstructor<R> compose(ComplexUnaryOperator<R> after) {
Review Comment:
`after` should be named `before`. However I think this compose function
should be removed. All the places you are using it you compose a new lambda
function for each call. IIUC if this is replaced with a fixed lambda function
(see the ComplexUnaryOperator for an example) then the lambda is a static
function and will be created once.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -80,6 +85,38 @@ public static void assertEquals(Complex expected, Complex
actual, double delta)
Assertions.assertEquals(expected.getImaginary(),
actual.getImaginary(), delta);
}
+ public static void assertEquals(Complex expected, Complex actual) {
Review Comment:
Uncommented method. The method actually checks the real and expected twice
because it uses Complex.equals. This method is only used in one place. It can
be removed as the alternative is to use Assertions.assertEquals.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+
+/**
+ * Represents a complex operation that accepts a complex number of type
ComplexDouble and produces a ComplexDouble result.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexUnaryOperator<R> {
+
+ /**
+ * Represents an operator that accepts a complex number and a complex
constructor to produce and return the result.
+ * @param in Complex number
+ * @param out Constructor
+ * @return ComplexDouble
+ */
+ default R apply(ComplexDouble in, ComplexConstructor<R> out) {
+ return apply(in.getReal(), in.getImaginary(), out);
+ }
+
+ /**
+ * Represents an operator that accepts real and imaginary parts of a
complex number and a complex constructor to produce and return the result.
+ * @param r real
+ * @param i imaginary
+ * @param out Constructor
+ * @return ComplexDouble
+ */
+ R apply(double r, double i, ComplexConstructor<R> out);
+
+ /**
+ * Returns a composed unary operator that first applies this function to
its input, and then applies the after UnaryOperator function to the result.
+ * If evaluation of either function throws an exception, it is relayed to
the caller of the composed function.
+ * @param after the function to apply after this function is applied
+ * @return a composed function that first applies this function and then
applies the after function
+ */
+ default ComplexUnaryOperator<R> andThen(ComplexUnaryOperator<R> after) {
+ Objects.requireNonNull(after);
+ return (real, imaginary, out) -> apply(real, imaginary,
out.compose(after));
Review Comment:
Avoid `compose` using:
```Java
return (real, imaginary, out) ->
apply(real, imaginary, (x, y) -> after.apply(x, y, out));
```
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexDouble.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+/**
+ * Cartesian representation of a complex number. The complex number is
expressed
+ * in the form \( a + ib \) where \( a \) and \( b \) are real numbers and \(
i \)
+ * is the imaginary unit which satisfies the equation \( i^2 = -1 \). For the
+ * complex number \( a + ib \), \( a \) is called the <em>real part</em> and
+ * \( b \) is called the <em>imaginary part</em>.
+ */
+public interface ComplexDouble {
Review Comment:
I do not think we need this interface. All the complex operators are defined
using a two part real and imaginary input; the output is defined with a
ComplexConstructor that accepts a real and imaginary.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexBinaryOperator.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+
+/**
+ * Represents an operation upon two operands of the same type, producing a
result of the same type as the operands.
+ * This is a specialization of BinaryOperator for the case where the operands
and the result are all of the same type.
+ * This is a functional interface whose functional method is
apply(ComplexDouble, ComplexDouble).
+ * @param <R> Generic
+*/
+@FunctionalInterface
+public interface ComplexBinaryOperator<R> {
+
+ /**
+ * Represents an operator that accepts two complex operands and a complex
constructor to produce and return the result.
+ * @param c1 Complex number 1
+ * @param c2 Complex number 2
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ default R apply(ComplexDouble c1, ComplexDouble c2, ComplexConstructor<R>
result) {
+ return apply(c1.getReal(), c1.getImaginary(), c2.getReal(),
c2.getImaginary(), result);
+ }
+
+ /**
+ * Represents an operator that accepts real and imaginary parts of two
complex operands and a complex constructor to produce and return the result.
+ * @param r1 real 1
+ * @param i1 imaginary 1
+ * @param r2 real 2
+ * @param i2 imaginary 2
+ * @param out constructor
+ * @return ComplexDouble
+ */
+ R apply(double r1, double i1, double r2, double i2, ComplexConstructor<R>
out);
+
+ /**
+ * Returns a composed function that first applies this function to its
input, and then applies the after function to the result.
+ * If evaluation of either function throws an exception, it is relayed to
the caller of the composed function.
+ * @param after the function to apply after this function is applied
+ * @return a composed function that first applies this function and then
applies the after function
+ */
+ default ComplexBinaryOperator<R> andThen(ComplexUnaryOperator<R> after) {
+ Objects.requireNonNull(after);
+ return (r1, i1, r2, i2, out) -> apply(r1, i1, r2, i2,
out.compose(after));
Review Comment:
Avoid `compose` using:
```Java
return (r1, i1, r2, i2, out) ->
apply(r1, i1, r2, i2, (x, y) -> after.apply(x, y, out));
```
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CReferenceTest.java:
##########
@@ -156,16 +156,22 @@ static void assertEquals(Supplier<String> msg, double
expected, double actual, l
*
* @param c Input number.
* @param name the operation name
- * @param operation the operation
+ * @param operation1 the Complex operation
+ * @param operation2 the ComplexFunctions operation
* @param expected Expected result.
* @param maxUlps the maximum units of least precision between the two
values
*/
static void assertComplex(Complex c,
- String name, UnaryOperator<Complex> operation,
- Complex expected, long maxUlps) {
- final Complex z = operation.apply(c);
+ String name, UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, long maxUlps) {
+ final Complex z = operation1.apply(c);
+
+ final ComplexDouble y = operation2.apply(c,
TestUtils.ComplexDoubleConstructor.of());
assertEquals(() -> c + "." + name + "(): real", expected.real(),
z.real(), maxUlps);
assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
z.imag(), maxUlps);
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
y.getReal(), maxUlps);
Review Comment:
Since you have two operations, you should use the type in the assertion
message:
```Java
assertEquals(() -> c + ". UnaryOperator " + name + "(): real",
expected.real(), z.real(), maxUlps);
assertEquals(() -> c + ". ComplexUnaryOperator " + name + "(): real",
expected.real(), z.real(), maxUlps);
```
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+
+/**
+ * Represents a complex operation that accepts a complex number of type
ComplexDouble and produces a ComplexDouble result.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexUnaryOperator<R> {
+
+ /**
+ * Represents an operator that accepts a complex number and a complex
constructor to produce and return the result.
+ * @param in Complex number
+ * @param out Constructor
+ * @return ComplexDouble
Review Comment:
`ComplexDouble` should be removed from all the function interfaces in the
javadoc. It is not used.
The docs can state that the function accepts a complex number composed of
real and imaginary parts and produces a result real and imaginary part that is
passed to the supplied constructor. The function returns the object created by
the supplied constructor.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -3285,413 +1681,42 @@ private static boolean equals(double x, double y) {
}
/**
- * Check that a value is negative. It must meet all the following
conditions:
- * <ul>
- * <li>it is not {@code NaN},</li>
- * <li>it is negative signed,</li>
- * </ul>
- *
- * <p>Note: This is true for negative zero.</p>
- *
- * @param d Value.
- * @return {@code true} if {@code d} is negative.
+ * This operator is used for all Complex operations that only deal with
one Complex number.
+ * @param operator ComplexUnaryOperator
+ * @return Complex
*/
- private static boolean negative(double d) {
- return d < 0 || Double.doubleToLongBits(d) == NEGATIVE_ZERO_LONG_BITS;
+ private Complex applyUnaryOperator(ComplexUnaryOperator<Complex> operator)
{
+ return operator.apply(this.real, this.imaginary, Complex::ofCartesian);
}
/**
- * Check that a value is positive infinity. Used to replace {@link
Double#isInfinite()}
- * when the input value is known to be positive (i.e. in the case where it
has been
- * set using {@link Math#abs(double)}).
- *
- * @param d Value.
- * @return {@code true} if {@code d} is +inf.
+ * This operator is used for all Complex operations that deals with two
Complex numbers.
+ * @param operator ComplexBinaryOperator
+ * @param input ComplexDouble
+ * @return Complex
*/
- private static boolean isPosInfinite(double d) {
- return d == Double.POSITIVE_INFINITY;
+ private Complex applyBinaryOperator(ComplexDouble input,
ComplexBinaryOperator<Complex> operator) {
+ return operator.apply(this.real, this.imaginary, input.getReal(),
input.getImaginary(), Complex::ofCartesian);
}
/**
- * Check that an absolute value is finite. Used to replace {@link
Double#isFinite(double)}
- * when the input value is known to be positive (i.e. in the case where it
has been
- * set using {@link Math#abs(double)}).
- *
- * @param d Value.
- * @return {@code true} if {@code d} is +finite.
+ * This operator is used for all Complex operations that deals with one
Complex number
+ * and a scalar factor.
+ * @param operator ComplexScalarFunction
+ * @param factor double
+ * @return Complex
*/
- private static boolean isPosFinite(double d) {
- return d <= Double.MAX_VALUE;
+ private Complex applyScalarFunction(double factor,
ComplexScalarFunction<Complex> operator) {
Review Comment:
I do not think these apply functions make the code nicer. Consider the
following:
```Java
public Complex exp() {
return applyUnaryOperator(ComplexFunctions::exp);
// or
return ComplexFunctions.exp(real, imaginary, Complex::ofCartesian);
}
public Complex multiply(Complex factor) {
return applyBinaryOperator(factor, ComplexFunctions::multiply);
// or
return ComplexFunctions.multiply(real, imaginary,
factor.real, factor.imaginary,
Complex::ofCartesian);
}
```
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+
+/**
+ * Represents an operation upon two operands of the different type, producing
a result.
+ * This is a functional interface whose functional method is {@link
#apply(double, double, double, ComplexConstructor)}.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexScalarFunction<R> {
+
+ /**
+ * Represents a binary function that accepts a Complex and a double
operand to produce a Complex result.
+ * The result is accepted by the ComplexConstructor.
+ * @param in Complex input
+ * @param operand the second function argument
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ default R apply(ComplexDouble in, double operand, ComplexConstructor<R>
result) {
+ return apply(in.getReal(), in.getImaginary(), operand, result);
+ }
+
+ /**
+ * Represents a binary function that accepts a Complex number's real and
imaginary parts
+ * and a double operand to produce a Complex result.
+ * The result is accepted by the ComplexConstructor.
+ * @param real part the first complex argument
+ * @param imaginary part of the first function argument
+ * @param operand the second function argument
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ R apply(double real, double imaginary, double operand,
ComplexConstructor<R> result);
+
+ /**
+ * Returns a composed scalar function that first applies this function to
its input, and then applies the after function to the result.
+ * If evaluation of either function throws an exception, it is relayed to
the caller of the composed function.
+ * @param after the function to apply after this function is applied
+ * @return a composed function that first applies this function and then
applies the after function
+ */
+ default ComplexScalarFunction<R> andThen(ComplexUnaryOperator<R> after) {
+ Objects.requireNonNull(after);
+ return (real, imaginary, operand, out) ->
Review Comment:
Avoid composition using:
```Java
return (real, imaginary, operand, out) ->
apply(real, imaginary, operand, (x, y) -> after.apply(x, y,
out));
```
In the original `compose` is invoked for each call to the returned lambda.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -3285,413 +1681,42 @@ private static boolean equals(double x, double y) {
}
/**
- * Check that a value is negative. It must meet all the following
conditions:
- * <ul>
- * <li>it is not {@code NaN},</li>
- * <li>it is negative signed,</li>
- * </ul>
- *
- * <p>Note: This is true for negative zero.</p>
- *
- * @param d Value.
- * @return {@code true} if {@code d} is negative.
+ * This operator is used for all Complex operations that only deal with
one Complex number.
+ * @param operator ComplexUnaryOperator
+ * @return Complex
*/
- private static boolean negative(double d) {
- return d < 0 || Double.doubleToLongBits(d) == NEGATIVE_ZERO_LONG_BITS;
+ private Complex applyUnaryOperator(ComplexUnaryOperator<Complex> operator)
{
+ return operator.apply(this.real, this.imaginary, Complex::ofCartesian);
}
/**
- * Check that a value is positive infinity. Used to replace {@link
Double#isInfinite()}
- * when the input value is known to be positive (i.e. in the case where it
has been
- * set using {@link Math#abs(double)}).
- *
- * @param d Value.
- * @return {@code true} if {@code d} is +inf.
+ * This operator is used for all Complex operations that deals with two
Complex numbers.
+ * @param operator ComplexBinaryOperator
+ * @param input ComplexDouble
+ * @return Complex
*/
- private static boolean isPosInfinite(double d) {
- return d == Double.POSITIVE_INFINITY;
+ private Complex applyBinaryOperator(ComplexDouble input,
ComplexBinaryOperator<Complex> operator) {
+ return operator.apply(this.real, this.imaginary, input.getReal(),
input.getImaginary(), Complex::ofCartesian);
}
/**
- * Check that an absolute value is finite. Used to replace {@link
Double#isFinite(double)}
- * when the input value is known to be positive (i.e. in the case where it
has been
- * set using {@link Math#abs(double)}).
- *
- * @param d Value.
- * @return {@code true} if {@code d} is +finite.
+ * This operator is used for all Complex operations that deals with one
Complex number
+ * and a scalar factor.
+ * @param operator ComplexScalarFunction
+ * @param factor double
+ * @return Complex
*/
- private static boolean isPosFinite(double d) {
- return d <= Double.MAX_VALUE;
+ private Complex applyScalarFunction(double factor,
ComplexScalarFunction<Complex> operator) {
+ return operator.apply(this.real, this.imaginary, factor,
Complex::ofCartesian);
}
/**
- * Create a complex number given the real and imaginary parts, then
multiply by {@code -i}.
- * This is used in functions that implement trigonomic identities. It is
the functional
- * equivalent of:
- *
- * <pre>
- * z = new Complex(real, imaginary).multiplyImaginary(-1);</pre>
- *
- * @param real Real part.
- * @param imaginary Imaginary part.
- * @return {@code Complex} object.
+ * This operator is used for all Complex operations that deals with one
Complex number
+ * but returns a double.
+ * @param operator DoubleBinaryOperator
+ * @return double
*/
- private static Complex multiplyNegativeI(double real, double imaginary) {
- return new Complex(imaginary, -real);
- }
-
- /**
- * Change the sign of the magnitude based on the signed value.
- *
- * <p>If the signed value is negative then the result is {@code
-magnitude}; otherwise
- * return {@code magnitude}.
- *
- * <p>A signed value of {@code -0.0} is treated as negative. A signed
value of {@code NaN}
- * is treated as positive.
- *
- * <p>This is not the same as {@link Math#copySign(double, double)} as
this method
- * will change the sign based on the signed value rather than copy the
sign.
- *
- * @param magnitude the magnitude
- * @param signedValue the signed value
- * @return magnitude or -magnitude.
- * @see #negative(double)
- */
- private static double changeSign(double magnitude, double signedValue) {
- return negative(signedValue) ? -magnitude : magnitude;
- }
-
- /**
- * Returns a scale suitable for use with {@link Math#scalb(double, int)}
to normalise
- * the number to the interval {@code [1, 2)}.
- *
- * <p>The scale is typically the largest unbiased exponent used in the
representation of the
- * two numbers. In contrast to {@link Math#getExponent(double)} this
handles
- * sub-normal numbers by computing the number of leading zeros in the
mantissa
- * and shifting the unbiased exponent. The result is that for all finite,
non-zero,
- * numbers {@code a, b}, the magnitude of {@code scalb(x, -getScale(a,
b))} is
- * always in the range {@code [1, 2)}, where {@code x = max(|a|, |b|)}.
- *
- * <p>This method is a functional equivalent of the c function
ilogb(double) adapted for
- * two input arguments.
- *
- * <p>The result is to be used to scale a complex number using {@link
Math#scalb(double, int)}.
- * Hence the special case of both zero arguments is handled using the
return value for NaN
- * as zero cannot be scaled. This is different from {@link
Math#getExponent(double)}
- * or {@link #getMaxExponent(double, double)}.
- *
- * <p>Special cases:
- *
- * <ul>
- * <li>If either argument is NaN or infinite, then the result is
- * {@link Double#MAX_EXPONENT} + 1.
- * <li>If both arguments are zero, then the result is
- * {@link Double#MAX_EXPONENT} + 1.
- * </ul>
- *
- * @param a the first value
- * @param b the second value
- * @return The maximum unbiased exponent of the values to be used for
scaling
- * @see Math#getExponent(double)
- * @see Math#scalb(double, int)
- * @see <a href="http://www.cplusplus.com/reference/cmath/ilogb/";>ilogb</a>
- */
- private static int getScale(double a, double b) {
- // Only interested in the exponent and mantissa so remove the sign bit
- final long x = Double.doubleToRawLongBits(a) & UNSIGN_MASK;
- final long y = Double.doubleToRawLongBits(b) & UNSIGN_MASK;
- // Only interested in the maximum
- final long bits = Math.max(x, y);
- // Get the unbiased exponent
- int exp = ((int) (bits >>> 52)) - EXPONENT_OFFSET;
-
- // No case to distinguish nan/inf
- // Handle sub-normal numbers
- if (exp == Double.MIN_EXPONENT - 1) {
- // Special case for zero, return as nan/inf to indicate scaling is
not possible
- if (bits == 0) {
- return Double.MAX_EXPONENT + 1;
- }
- // A sub-normal number has an exponent below -1022. The amount
below
- // is defined by the number of shifts of the most significant bit
in
- // the mantissa that is required to get a 1 at position 53 (i.e. as
- // if it were a normal number with assumed leading bit)
- final long mantissa = bits & MANTISSA_MASK;
- exp -= Long.numberOfLeadingZeros(mantissa << 12);
- }
- return exp;
- }
-
- /**
- * Returns the largest unbiased exponent used in the representation of the
- * two numbers. Special cases:
- *
- * <ul>
- * <li>If either argument is NaN or infinite, then the result is
- * {@link Double#MAX_EXPONENT} + 1.
- * <li>If both arguments are zero or subnormal, then the result is
- * {@link Double#MIN_EXPONENT} -1.
- * </ul>
- *
- * <p>This is used by {@link #divide(double, double, double, double)} as
- * a simple detection that a number may overflow if multiplied
- * by a value in the interval [1, 2).
- *
- * @param a the first value
- * @param b the second value
- * @return The maximum unbiased exponent of the values.
- * @see Math#getExponent(double)
- * @see #divide(double, double, double, double)
- */
- private static int getMaxExponent(double a, double b) {
- // This could return:
- // Math.getExponent(Math.max(Math.abs(a), Math.abs(b)))
- // A speed test is required to determine performance.
- return Math.max(Math.getExponent(a), Math.getExponent(b));
- }
-
- /**
- * Checks if both x and y are in the region defined by the minimum and
maximum.
- *
- * @param x x value.
- * @param y y value.
- * @param min the minimum (exclusive).
- * @param max the maximum (exclusive).
- * @return true if inside the region.
- */
- private static boolean inRegion(double x, double y, double min, double
max) {
- return x < max && x > min && y < max && y > min;
- }
-
- /**
- * Returns {@code sqrt(x^2 + y^2)} without intermediate overflow or
underflow.
- *
- * <p>Special cases:
- * <ul>
- * <li>If either argument is infinite, then the result is positive
infinity.
- * <li>If either argument is NaN and neither argument is infinite, then
the result is NaN.
- * </ul>
- *
- * <p>The computed result is expected to be within 1 ulp of the exact
result.
- *
- * <p>This method is a replacement for {@link Math#hypot(double, double)}.
There
- * will be differences between this method and {@code Math.hypot(double,
double)} due
- * to the use of a different algorithm to compute the high precision sum of
- * {@code x^2 + y^2}. This method has been tested to have a lower maximum
error from
- * the exact result; any differences are expected to be 1 ULP indicating a
rounding
- * change in the sum.
- *
- * <p>JDK9 ported the hypot function to Java for bug JDK-7130085 due to
the slow performance
- * of the method as a native function. Benchmarks of the Complex class for
functions that
- * use hypot confirm this is slow pre-Java 9. This implementation
outperforms the new faster
- * {@code Math.hypot(double, double)} on JDK 11 (LTS). See the Commons
numbers examples JMH
- * module for benchmarks. Comparisons with alternative implementations
indicate
- * performance gains are related to edge case handling and elimination of
an unpredictable
- * branch in the computation of {@code x^2 + y^2}.
- *
- * <p>This port was adapted from the "Freely Distributable Math Library"
hypot function.
- * This method only (and not invoked methods within) is distributed under
the terms of the
- * original notice as shown below:
- * <pre>
- * ====================================================
- * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
- *
- * Developed at SunSoft, a Sun Microsystems, Inc. business.
- * Permission to use, copy, modify, and distribute this
- * software is freely granted, provided that this notice
- * is preserved.
- * ====================================================
- * </pre>
- *
- * <p>Note: The fdlibm c code makes use of the language ability to read
and write directly
- * to the upper and lower 32-bits of the 64-double. The function performs
- * checking on the upper 32-bits for the magnitude of the two numbers by
accessing
- * the exponent and 20 most significant bits of the mantissa. These upper
bits
- * are manipulated during scaling and then used to perform extended
precision
- * computation of the sum {@code x^2 + y^2} where the high part of the
number has 20-bit
- * precision. Manipulation of direct bits has no equivalent in Java
- * other than use of {@link Double#doubleToLongBits(double)} and
- * {@link Double#longBitsToDouble(long)}. To avoid conversion to and from
long and double
- * representations this implementation only scales the double
representation. The high
- * and low parts of a double for the extended precision computation are
extracted
- * using the method of Dekker (1971) to create two 26-bit numbers. This
works for sub-normal
- * numbers and reduces the maximum error in comparison to fdlibm hypot
which does not
- * use a split number algorithm for sub-normal numbers.
- *
- * @param x Value x
- * @param y Value y
- * @return sqrt(x^2 + y^2)
- * @see Math#hypot(double, double)
- * @see <a href="https://www.netlib.org/fdlibm/e_hypot.c";>fdlibm
e_hypot.c</a>
- * @see <a
href="https://bugs.java.com/bugdatabase/view_bug.do?bug_id=7130085";>JDK-7130085
: Port fdlibm hypot to Java</a>
- */
- private static double hypot(double x, double y) {
- // Differences to the fdlibm reference:
- //
- // 1. fdlibm orders the two parts using the magnitude of the upper
32-bits.
- // This incorrectly orders numbers which differ only in the lower
32-bits.
- // This invalidates hypot(x, y) = hypot(y, x) for small sub-normal
numbers and a minority
- // of cases of normal numbers. This implementation forces the |x| >=
|y| order
- // using the entire 63-bits of the unsigned doubles to ensure the
function
- // is commutative.
- //
- // 2. fdlibm computed scaling by directly writing changes to the
exponent bits
- // and maintained the high part (ha) during scaling for use in the high
- // precision sum x^2 + y^2. Since exponent scaling cannot be applied
to sub-normals
- // the original version dropped the split number representation for
sub-normals
- // and can produce maximum errors above 1 ULP for sub-normal numbers.
- // This version uses Dekker's method to split the number. This can be
applied to
- // sub-normals and allows dropping the condition to check for
sub-normal numbers
- // since all small numbers are handled with a single scaling factor.
- // The effect is increased precision for the majority of sub-normal
cases where
- // the implementations compute a different result.
- //
- // 3. An alteration is done here to add an 'else if' instead of a
second
- // 'if' statement. Thus you cannot scale down and up at the same time.
- //
- // 4. There is no use of the absolute double value. The magnitude
comparison is
- // performed using the long bit representation. The computation
x^2+y^2 is
- // insensitive to the sign bit. Thus use of Math.abs(double) is only
in edge-case
- // branches.
- //
- // 5. The exponent different to ignore the smaller component has
changed from 60 to 54.
- //
- // Original comments from fdlibm are in c style: /* */
- // Extra comments added for reference.
- //
- // Note that the high 32-bits are compared to constants.
- // The lowest 20-bits are the upper bits of the 52-bit mantissa.
- // The next 11-bits are the biased exponent. The sign bit has been
cleared.
- // Scaling factors are powers of two for exact scaling.
- // For clarity the values have been refactored to named constants.
-
- // The mask is used to remove the sign bit.
- final long xbits = Double.doubleToRawLongBits(x) & UNSIGN_MASK;
- final long ybits = Double.doubleToRawLongBits(y) & UNSIGN_MASK;
-
- // Order by magnitude: |a| >= |b|
- double a;
- double b;
- /* High word of x & y */
- int ha;
- int hb;
- if (ybits > xbits) {
- a = y;
- b = x;
- ha = (int) (ybits >>> 32);
- hb = (int) (xbits >>> 32);
- } else {
- a = x;
- b = y;
- ha = (int) (xbits >>> 32);
- hb = (int) (ybits >>> 32);
- }
-
- // Check if the smaller part is significant.
- // a^2 is computed in extended precision for an effective mantissa of
106-bits.
- // An exponent difference of 54 is where b^2 will not overlap a^2.
- if ((ha - hb) > EXP_54) {
- /* a/b > 2**54 */
- // or a is Inf or NaN.
- // No addition of a + b for sNaN.
- return Math.abs(a);
- }
-
- double rescale = 1.0;
- if (ha > EXP_500) {
- /* a > 2^500 */
- if (ha >= EXP_1024) {
- /* Inf or NaN */
- // Check b is infinite for the IEEE754 result.
- // No addition of a + b for sNaN.
- return Math.abs(b) == Double.POSITIVE_INFINITY ?
- Double.POSITIVE_INFINITY :
- Math.abs(a);
- }
- /* scale a and b by 2^-600 */
- // Before scaling: a in [2^500, 2^1023].
- // After scaling: a in [2^-100, 2^423].
- // After scaling: b in [2^-154, 2^423].
- a *= TWO_POW_NEG_600;
- b *= TWO_POW_NEG_600;
- rescale = TWO_POW_600;
- } else if (hb < EXP_NEG_500) {
- // No special handling of sub-normals.
- // These do not matter when we do not manipulate the exponent bits
- // for scaling the split representation.
-
- // Intentional comparison with zero.
- if (b == 0) {
- return Math.abs(a);
- }
-
- /* scale a and b by 2^600 */
- // Effective min exponent of a sub-normal = -1022 - 52 = -1074.
- // Before scaling: b in [2^-1074, 2^-501].
- // After scaling: b in [2^-474, 2^99].
- // After scaling: a in [2^-474, 2^153].
- a *= TWO_POW_600;
- b *= TWO_POW_600;
- rescale = TWO_POW_NEG_600;
- }
-
- // High precision x^2 + y^2
- return Math.sqrt(x2y2(a, b)) * rescale;
- }
-
- /**
- * Return {@code x^2 + y^2} with high accuracy.
- *
- * <p>It is assumed that {@code 2^500 > |x| >= |y| > 2^-500}. Thus there
will be no
- * overflow or underflow of the result. The inputs are not assumed to be
unsigned.
- *
- * <p>The computation is performed using Dekker's method for extended
precision
- * multiplication of x and y and then summation of the extended precision
squares.
- *
- * @param x Value x.
- * @param y Value y
- * @return x^2 + y^2
- * @see <a href="https://doi.org/10.1007/BF01397083";>
- * Dekker (1971) A floating-point technique for extending the available
precision</a>
- */
- private static double x2y2(double x, double y) {
- // Note:
- // This method is different from the high-accuracy summation used in
fdlibm for hypot.
- // The summation could be any valid computation of x^2+y^2. However
since this follows
- // the re-scaling logic in hypot(x, y) the use of high precision has
relatively
- // less performance overhead than if used without scaling.
- // The Dekker algorithm is branchless for better performance
- // than the fdlibm method with a maximum ULP error of approximately
0.86.
- //
- // See NUMBERS-143 for analysis.
-
- // Do a Dekker summation of double length products x*x and y*y
- // (10 multiply and 20 additions).
- final double xx = x * x;
- final double yy = y * y;
- // Compute the round-off from the products.
- // With FMA hardware support in JDK 9+ this can be replaced with the
much faster:
- // xxLow = Math.fma(x, x, -xx)
- // yyLow = Math.fma(y, y, -yy)
- // Dekker mul12
- final double xHigh = splitHigh(x);
- final double xLow = x - xHigh;
- final double xxLow = squareLow(xLow, xHigh, xx);
- // Dekker mul12
- final double yHigh = splitHigh(y);
- final double yLow = y - yHigh;
- final double yyLow = squareLow(yLow, yHigh, yy);
- // Dekker add2
- final double r = xx + yy;
- // Note: The order is important. Assume xx > yy and drop Dekker's
conditional
- // check for which is the greater magnitude.
- // s = xx - r + yy + yyLow + xxLow
- // z = r + s
- // zz = r - z + s
- // Here we compute z inline and ignore computing the round-off zz.
- // Note: The round-off could be used with Dekker's sqrt2 method.
- // That adds 7 multiply, 1 division and 19 additions doubling the cost
- // and reducing error to < 0.5 ulp for the final sqrt.
- return xx - r + yy + yyLow + xxLow + r;
+ private double applyToDoubleFunction(DoubleBinaryOperator operator) {
Review Comment:
This is totally not required. It can be replaced by calling
`ComplexFunctions.xxx(r, i)`
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
Review Comment:
There are many test methods with a delta and a comment stating the values
must be binary equal. This is not true.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+
+/**
+ * Represents a complex operation that accepts a complex number of type
ComplexDouble and produces a ComplexDouble result.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexUnaryOperator<R> {
+
+ /**
+ * Represents an operator that accepts a complex number and a complex
constructor to produce and return the result.
+ * @param in Complex number
+ * @param out Constructor
+ * @return ComplexDouble
+ */
+ default R apply(ComplexDouble in, ComplexConstructor<R> out) {
Review Comment:
For now I think we can drop `ComplexDouble` and these default functions.
They can be added later if required since they are defaults. For now all the
code I think can be updated to directly use the (r, i) accepting apply function.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexFunctions.java:
##########
@@ -0,0 +1,2737 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.numbers.complex;
+
+import java.util.Objects;
+import java.util.function.DoubleUnaryOperator;
+
+/**
+ * <p>This class is all the Complex arithmetics. All the arithmetics that uses
1 or 2 Complex
+ * number to produce a Complex result.
+ * All arithmetic will create a new instance for the result.</p>
+ */
+public final class ComplexFunctions {
+
+ /** The bit representation of {@code -0.0}. */
+ static final long NEGATIVE_ZERO_LONG_BITS = Double.doubleToLongBits(-0.0);
+ /** Exponent offset in IEEE754 representation. */
+ static final int EXPONENT_OFFSET = 1023;
+
+ /** Mask to remove the sign bit from a long. */
+ static final long UNSIGN_MASK = 0x7fff_ffff_ffff_ffffL;
+ /** Mask to extract the 52-bit mantissa from a long representation of a
double. */
+ static final long MANTISSA_MASK = 0x000f_ffff_ffff_ffffL;
+
+ /** π/2. */
+ private static final double PI_OVER_2 = 0.5 * Math.PI;
+ /** π/4. */
+ private static final double PI_OVER_4 = 0.25 * Math.PI;
+ /** Natural logarithm of 2 (ln(2)). */
+ private static final double LN_2 = Math.log(2);
+ /** {@code 1/2}. */
+ private static final double HALF = 0.5;
+ /** Base 10 logarithm of 10 divided by 2 (log10(e)/2). */
+ private static final double LOG_10E_O_2 = Math.log10(Math.E) / 2;
+ /** Base 10 logarithm of 2 (log10(2)). */
+ private static final double LOG10_2 = Math.log10(2);
+ /** {@code sqrt(2)}. */
+ private static final double ROOT2 = 1.4142135623730951;
+ /** {@code 1.0 / sqrt(2)}.
+ * This is pre-computed to the closest double from the exact result.
+ * It is 1 ULP different from 1.0 / Math.sqrt(2) but equal to Math.sqrt(2)
/ 2.
+ */
+ private static final double ONE_OVER_ROOT2 = 0.7071067811865476;
+
+ /**
+ * Largest double-precision floating-point number such that
+ * {@code 1 + EPSILON} is numerically equal to 1. This value is an upper
+ * bound on the relative error due to rounding real numbers to double
+ * precision floating-point numbers.
+ *
+ * <p>In IEEE 754 arithmetic, this is 2<sup>-53</sup>.
+ * Copied from o.a.c.numbers.Precision.
+ *
+ * @see <a href="http://en.wikipedia.org/wiki/Machine_epsilon";>Machine
epsilon</a>
+ */
+ private static final double EPSILON =
Double.longBitsToDouble((EXPONENT_OFFSET - 53L) << 52);
+
+ /** The multiplier used to split the double value into hi and low parts.
This must be odd
+ * and a value of 2^s + 1 in the range {@code p/2 <= s <= p-1} where p is
the number of
+ * bits of precision of the floating point number. Here {@code s = 27}.*/
+ private static final double MULTIPLIER = 1.34217729E8;
+
+ /**
+ * Crossover point to switch computation for asin/acos factor A.
+ * This has been updated from the 1.5 value used by Hull et al to 10
+ * as used in boost::math::complex.
+ * @see <a href="https://svn.boost.org/trac/boost/ticket/7290";>Boost
ticket 7290</a>
+ */
+ private static final double A_CROSSOVER = 10.0;
+ /** Crossover point to switch computation for asin/acos factor B. */
+ private static final double B_CROSSOVER = 0.6471;
+ /**
+ * The safe maximum double value {@code x} to avoid loss of precision in
asin/acos.
+ * Equal to sqrt(M) / 8 in Hull, et al (1997) with M the largest
normalised floating-point value.
+ */
+ private static final double SAFE_MAX = Math.sqrt(Double.MAX_VALUE) / 8;
+ /**
+ * The safe minimum double value {@code x} to avoid loss of
precision/underflow in asin/acos.
+ * Equal to sqrt(u) * 4 in Hull, et al (1997) with u the smallest
normalised floating-point value.
+ */
+ private static final double SAFE_MIN = Math.sqrt(Double.MIN_NORMAL) * 4;
+ /**
+ * The safe maximum double value {@code x} to avoid loss of precision in
atanh.
+ * Equal to sqrt(M) / 2 with M the largest normalised floating-point value.
+ */
+ private static final double SAFE_UPPER = Math.sqrt(Double.MAX_VALUE) / 2;
+ /**
+ * The safe minimum double value {@code x} to avoid loss of
precision/underflow in atanh.
+ * Equal to sqrt(u) * 2 with u the smallest normalised floating-point
value.
+ */
+ private static final double SAFE_LOWER = Math.sqrt(Double.MIN_NORMAL) * 2;
+ /** The safe maximum double value {@code x} to avoid overflow in sqrt. */
+ private static final double SQRT_SAFE_UPPER = Double.MAX_VALUE / 8;
+ /**
+ * A safe maximum double value {@code m} where {@code e^m} is not infinite.
+ * This can be used when functions require approximations of sinh(x) or
cosh(x)
+ * when x is large using exp(x):
+ * <pre>
+ * sinh(x) = (e^x - e^-x) / 2 = sign(x) * e^|x| / 2
+ * cosh(x) = (e^x + e^-x) / 2 = e^|x| / 2 </pre>
+ *
+ * <p>This value can be used to approximate e^x using a product:
+ *
+ * <pre>
+ * e^x = product_n (e^m) * e^(x-nm)
+ * n = (int) x/m
+ * e.g. e^2000 = e^m * e^m * e^(2000 - 2m) </pre>
+ *
+ * <p>The value should be below ln(max_value) ~ 709.783.
+ * The value m is set to an integer for less error when subtracting m and
chosen as
+ * even (m=708) as it is used as a threshold in tanh with m/2.
+ *
+ * <p>The value is used to compute e^x multiplied by a small number
avoiding
+ * overflow (sinh/cosh) or a small number divided by e^x without underflow
due to
+ * infinite e^x (tanh). The following conditions are used:
+ * <pre>
+ * 0.5 * e^m * Double.MIN_VALUE * e^m * e^m = Infinity
+ * 2.0 / e^m / e^m = 0.0 </pre>
+ */
+ private static final double SAFE_EXP = 708;
+ /**
+ * The value of Math.exp(SAFE_EXP): e^708.
+ * To be used in overflow/underflow safe products of e^m to approximate
e^x where x > m.
+ */
+ private static final double EXP_M = Math.exp(SAFE_EXP);
+
+ /** 54 shifted 20-bits to align with the exponent of the upper 32-bits of
a double. */
+ private static final int EXP_54 = 0x36_00000;
+ /** Represents an exponent of 500 in unbiased form shifted 20-bits to
align with the upper 32-bits of a double. */
+ private static final int EXP_500 = 0x5f3_00000;
+ /** Represents an exponent of 1024 in unbiased form (infinite or nan)
+ * shifted 20-bits to align with the upper 32-bits of a double. */
+ private static final int EXP_1024 = 0x7ff_00000;
+ /** Represents an exponent of -500 in unbiased form shifted 20-bits to
align with the upper 32-bits of a double. */
+ private static final int EXP_NEG_500 = 0x20b_00000;
+ /** 2^600. */
+ private static final double TWO_POW_600 = 0x1.0p+600;
+ /** 2^-600. */
+ private static final double TWO_POW_NEG_600 = 0x1.0p-600;
+
+ /**
+ * Private constructor for utility class.
+ */
+ private ComplexFunctions() {
+
+ }
+
+ /**
+ * Returns the absolute value of the complex number.
+ * <pre>abs(x + i y) = sqrt(x^2 + y^2)</pre>
+ *
+ * <p>This should satisfy the special cases of the hypot function in ISO
C99 F.9.4.3:
+ * "The hypot functions compute the square root of the sum of the squares
of x and y,
+ * without undue overflow or underflow."
+ *
+ * <ul>
+ * <li>hypot(x, y), hypot(y, x), and hypot(x, −y) are equivalent.
+ * <li>hypot(x, ±0) is equivalent to |x|.
+ * <li>hypot(±∞, y) returns +∞, even if y is a NaN.
+ * </ul>
+ *
+ * <p>This method is called by all methods that require the absolute value
of the complex
+ * number, e.g. abs(), sqrt() and log().
+ *
+ * @param real Real part.
+ * @param imaginary Imaginary part.
+ * @return The absolute value.
+ */
+ public static double abs(double real, double imaginary) {
+ // Specialised implementation of hypot.
+ // See NUMBERS-143
+ return hypot(real, imaginary);
+ }
+
+ /**
+ * Returns the argument of this complex number.
+ *
+ * <p>The argument is the angle phi between the positive real axis and
+ * the point representing this number in the complex plane.
+ * The value returned is between \( -\pi \) (not inclusive)
+ * and \( \pi \) (inclusive), with negative values returned for numbers
with
+ * negative imaginary parts.
+ *
+ * <p>If either real or imaginary part (or both) is NaN, then the result
is NaN.
+ * Infinite parts are handled as {@linkplain Math#atan2} handles them,
+ * essentially treating finite parts as zero in the presence of an
+ * infinite coordinate and returning a multiple of \( \frac{\pi}{4} \)
depending on
+ * the signs of the infinite parts.
+ *
+ * <p>This code follows the
+ * <a href="http://www.iso-9899.info/wiki/The_Standard";>ISO C
Standard</a>, Annex G,
+ * in calculating the returned value using the {@code atan2(y, x)} method
for complex
+ * \( x + iy \).
+ *
+ * @param r real
+ * @param i imaginary
+ * @return The argument of this complex number.
+ * @see Math#atan2(double, double)
+ */
+ public static double arg(double r, double i) {
+ // Delegate
+ return Math.atan2(i, r);
+ }
+
+ /**
+ * Returns the squared norm value of this complex number. This is also
called the absolute
+ * square.
+ *
+ * <p>\[ \text{norm}(x + i y) = x^2 + y^2 \]
+ *
+ * <p>If either component is infinite then the result is positive
infinity. If either
+ * component is NaN and this is not {@link #isInfinite(ComplexDouble)
infinite} then the result is NaN.
+ *
+ * <p>Note: This method may not return the same value as the square of
{@link #abs(double, double)} as
+ * that method uses an extended precision computation.
+ *
+ * <p>{@code norm()} can be used as a faster alternative than {@code
abs()} for ranking by
+ * magnitude. If used for ranking any overflow to infinity will create an
equal ranking for
+ * values that may be still distinguished by {@code abs()}.
+ *
+ * @param real real part
+ * @param imaginary imaginary part
+ * @return The square norm value.
+ * @see #isInfinite(ComplexDouble)
+ * @see #abs(double, double)
+ * @see <a
href="http://mathworld.wolfram.com/AbsoluteSquare.html";>Absolute square</a>
+ */
+ public static double norm(double real, double imaginary) {
+ if (isInfinite(real, imaginary)) {
+ return Double.POSITIVE_INFINITY;
+ }
+ return real * real + imaginary * imaginary;
+ }
+
+ /**
+ * Returns {@code true} if either the real <em>or</em> imaginary component
of the complex number is NaN
+ * <em>and</em> the complex number is not infinite.
+ *
+ * <p>Note that:
+ * <ul>
+ * <li>There is more than one complex number that can return {@code
true}.
+ * <li>Different representations of NaN can be distinguished by the
+ * {@link #equals(Object) ComplexDouble.equals(Object)} method.
+ * </ul>
+ *
+ * @param complex number to check if is not a number
+ * @return {@code true} if this instance contains NaN and no infinite
parts.
+ * @see Double#isNaN(double)
+ * @see #isInfinite(ComplexDouble)
+ * @see #equals(Object) Complex.equals(Object)
+ */
+ public static boolean isNaN(ComplexDouble complex) {
+ if (Double.isNaN(complex.getReal()) ||
Double.isNaN(complex.getImaginary())) {
+ return !isInfinite(complex);
+ }
+ return false;
+ }
+
+ /**
+ * Returns {@code true} if either real or imaginary component of the
complex number is infinite.
+ *
+ * <p>Note: A complex number with at least one infinite part is regarded
+ * as an infinity (even if its other part is a NaN).
+ * @param c Complex number
+ * @return {@code true} if this instance contains an infinite value.
+ * @see Double#isInfinite(double)
+ */
+ public static boolean isInfinite(ComplexDouble c) {
+ return isInfinite(c.getReal(), c.getImaginary());
+ }
+
+ /**
+ * Returns {@code true} if either real or imaginary component of the
complex number is infinite.
+ *
+ * <p>Note: A complex number with at least one infinite part is regarded
+ * as an infinity (even if its other part is a NaN).
+ * @param real real
+ * @param imaginary imaginary
+ * @return {@code true} if this instance contains an infinite value.
+ * @see Double#isInfinite(double)
+ */
+ private static boolean isInfinite(double real, double imaginary) {
+ return Double.isInfinite(real) || Double.isInfinite(imaginary);
+ }
+
+ /**
+ *
+ * Returns {@code true} if both real and imaginary component of the
complex number are finite.
+ * @param complex number to check if is finite
+ * @return {@code true} if this instance contains finite values.
+ * @see Double#isFinite(double)
+ */
+ public static boolean isFinite(ComplexDouble complex) {
+ return Double.isFinite(complex.getReal()) &&
Double.isFinite(complex.getImaginary());
+ }
+
+ /**
+ * Returns a {@code Complex} whose value is the negation of both the real
and imaginary parts
+ * of complex number \( z \).
+ *
+ * <p>\[ \begin{aligned}
+ * z &= a + i b \\
+ * -z &= -a - i b \end{aligned} \]
+ *
+ * @param r real
+ * @param i imaginary
+ * @param result Constructor
+ * @return \( -z \).
+ * @param <R> generic
+ */
+ public static <R> R negate(double r, double i, ComplexConstructor<R>
result) {
+ return result.apply(-r, -i);
+ }
+
+ /**
+ * Returns the
+ * <a
href="http://mathworld.wolfram.com/ComplexConjugate.html";>conjugate</a>
+ * \( \overline{z} \) of this complex number \( z \) using its real and
imaginary parts.
+ *
+ * <p>\[ \begin{aligned}
+ * z &= a + i b \\
+ * \overline{z} &= a - i b \end{aligned}\]
+ *
+ * @param r real
+ * @param i imaginary
+ * @param result Constructor
+ * @return The conjugate (\( \overline{z} \)) of this complex number.
+ * @param <R> generic
+ */
+ public static <R> R conj(double r, double i, ComplexConstructor<R> result)
{
+ return result.apply(r, -i);
+ }
+
+ /**
+ * Returns a {@code Complex} whose value is obtained using the real and
Review Comment:
The javadoc for all this class requires work since it has been copied from
the Complex class with little change.
- Any references to `{@code this}` should be replaced with `the input
complex`
- The functions do not return a Complex, they return the object created by
the supplied constructor
- The `@param` tags should be more descriptive, e.g. the real part of the
first complex number, etc.
- <R> should be documented as the object type produced by the supplied
constructor
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
Review Comment:
All these methods have been copied from `CStandardTest` including the
comments about satisfying the conjugate equality. This is not actually tested
in the methods so please remove.
I think that given the methods are all overloaded they can be rename to
`assertEquals` (with delta) and `assertSame` (no delta).
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name, double deltaReal, double deltaImaginary) {
+
+ final ComplexDouble result = operation.apply(c, operand,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
actual.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
actual.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the double operation on the complex number is equal to the
expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertDouble(Complex c, DoubleBinaryOperator operation,
+ double expected, String name) {
+ assertDouble(c.getReal(), c.getImaginary(), operation, expected, name,
0.0D);
+ }
+
+ /**
+ * Assert the double operation on the complex number (real and imag parts)
is equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param r real
+ * @param i imaginary
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param delta delta
+ */
+ static void assertDouble(double r, double i, DoubleBinaryOperator
operation,
Review Comment:
This function should be changed to accept two operations:
`java.util.function.ToDoubleFunction<Complex> `and `DoubleBinaryOperator`.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name, double deltaReal, double deltaImaginary) {
+
+ final ComplexDouble result = operation.apply(c, operand,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
actual.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
actual.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result.getReal(), deltaReal);
Review Comment:
We wish to test that the two operations produce the exact same result. You
are not doing this here. Imagine:
```
a = f(x)
b = g(x)
test a == b // must compute the exact same result
test a ~ expected // must be close the expected answer
```
The assertion failure message should reflect what has failed
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CStandardTest.java:
##########
@@ -194,26 +194,31 @@ private static boolean negative(double d) {
* @param c1 the first complex (actual)
* @param c2 the second complex (expected)
*/
- private static void assertComplex(Complex c1, Complex c2) {
+ private static void assertComplex(ComplexDouble c1, ComplexDouble c2) {
// Use a delta of zero to allow comparison of -0.0 to 0.0
Assertions.assertEquals(c2.getReal(), c1.getReal(), 0.0, "real");
Assertions.assertEquals(c2.getImaginary(), c1.getImaginary(), 0.0,
"imaginary");
}
/**
* Assert the operation on the two complex numbers.
- *
- * @param c1 the first complex
+ * @param c1 the first complex
* @param c2 the second complex
- * @param operation the operation
+ * @param operation1 the Complex operation
+ * @param operation2 ComplexFunctions operation
* @param operationName the operation name
* @param expected the expected
* @param expectedName the expected name
*/
private static void assertOperation(Complex c1, Complex c2,
- BiFunction<Complex, Complex, Complex> operation, String
operationName,
- Predicate<Complex> expected, String expectedName) {
- final Complex z = operation.apply(c1, c2);
+ BiFunction<Complex, Complex, Complex>
operation1,
Review Comment:
Here you have updated line indentation so the diff is harder to read. Please
look at the `files changed` tab in your PR to see the full diff and make sure
there are not unnecessary changes. It is not always required to indent
arguments to the `(` of the method, especially when the method name and
argument lists are long.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name, double deltaReal, double deltaImaginary) {
+
+ final ComplexDouble result = operation.apply(c, operand,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
actual.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
actual.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the double operation on the complex number is equal to the
expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertDouble(Complex c, DoubleBinaryOperator operation,
+ double expected, String name) {
+ assertDouble(c.getReal(), c.getImaginary(), operation, expected, name,
0.0D);
+ }
+
+ /**
+ * Assert the double operation on the complex number (real and imag parts)
is equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param r real
+ * @param i imaginary
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param delta delta
+ */
+ static void assertDouble(double r, double i, DoubleBinaryOperator
operation,
+ double expected, String name, double delta) {
+
+ final double result = operation.applyAsDouble(r, i);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + expected + "):
result", expected, result, delta);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name) {
+ assertComplexUnary(c, operation1, operation2, expected, name, 0.0D,
0.0D);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param delta delta
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name, double
delta) {
+ assertComplexUnary(c, operation1, operation2, expected, name, delta,
delta);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name, double
deltaReal, double deltaImaginary) {
+ final Complex result1 = operation1.apply(c);
+ final ComplexDouble result2 = operation2.apply(c,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
result1.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
result1.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result2.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result2.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the binary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c1 the first complex
+ * @param c2 the second complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertComplexBinary(Complex c1, Complex c2,
+ BinaryOperator<Complex> operation1,
ComplexBinaryOperator<ComplexDouble> operation2,
+ Complex expected, String name) {
+ assertComplexBinary(c1, c2, operation1, operation2, expected, name,
0.0D, 0.0D);
Review Comment:
For complex number tests do not call an equals method with a tolerance of
`0.0`. This will fail to detect a difference between `-0.0` and `0.0`. This is
**critical** for testing branch cuts in the ISO c99 functions.
This method should duplicate the tests in the equals method that uses a
delta; it should be renamed to `assertSame`.
- assertSame must have binary equality (including signed zeros).
- assertEquals should be within the provided delta. A delta of zero will
allow zeros to match.
Note: The method this javadoc was originally copied from states this binary
equality test is made. So please make sure it is maintained.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class ComplexComposeTest {
+
+ private static final ComplexUnaryOperator<ComplexDouble> neg =
ComplexFunctions::negate;
+ private static final ComplexUnaryOperator<ComplexDouble> multiplyImag =
ComplexFunctions::multiplyImaginary;
+ private static final ComplexUnaryOperator<ComplexDouble> conj =
ComplexFunctions::conj;
+ private static final ComplexUnaryOperator<ComplexDouble> multiplyImagConj
= multiplyImag.andThen(conj);
+ private static final ComplexUnaryOperator<ComplexDouble> conjMultiplyImag
= conj.andThen(multiplyImag);
+ private static final ComplexUnaryOperator<ComplexDouble> identity1 =
multiplyImagConj.andThen(multiplyImagConj);
+ private static final ComplexUnaryOperator<ComplexDouble> identity2 =
conjMultiplyImag.andThen(conjMultiplyImag);
+
+ @Test
+ void testConjugateIdentities() {
Review Comment:
This type of test is better as a `@ParameterizedTest` using a `@CsvSource`
to provide the (r, i) pairs. Since the same numbers are used for two tests you
could use a `@MethodSource` for the two test methods that require a single
number:
```Java
static Stream<Arguments> singleComplex() {
return Stream.of(
Arguments.of(3, 4),
Arguments.of(3, -4)
);
}
@ParameterizedTest
@MethodSource(value = {"singleComplex"})
void testConjugateIdentity(double r, double i) {
Complex c = Complex.ofCartesian(r, i);
```
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CStandardTest.java:
##########
@@ -194,26 +194,31 @@ private static boolean negative(double d) {
* @param c1 the first complex (actual)
* @param c2 the second complex (expected)
*/
- private static void assertComplex(Complex c1, Complex c2) {
+ private static void assertComplex(ComplexDouble c1, ComplexDouble c2) {
Review Comment:
This method may be redundant now. The tests should be updated to call
`TestUtils.assertXXX` with two operations: one using the methods within
Complex, and one using methods in ComplexFunctions. The functions may require
composition.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name, double deltaReal, double deltaImaginary) {
+
+ final ComplexDouble result = operation.apply(c, operand,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
actual.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
actual.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the double operation on the complex number is equal to the
expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertDouble(Complex c, DoubleBinaryOperator operation,
Review Comment:
This function should be changed to accept two operations:
`java.util.function.ToDoubleFunction<Complex> `and `DoubleBinaryOperator`.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexFunctionsTest.java:
##########
@@ -0,0 +1,1175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import org.apache.commons.rng.UniformRandomProvider;
+import org.apache.commons.rng.simple.RandomSource;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.function.BiFunction;
+import java.util.function.DoubleFunction;
+import java.util.function.Supplier;
+
+/**
+ * Tests for {@link Complex} and {@link ComplexFunctions}.
+ *
+ * <p>Note: The ISO C99 math functions are not fully tested in this class. See
also:
+ *
+ * <ul>
+ * <li>{@link CStandardTest} for a test of the ISO C99 standards including
special case handling.
+ * <li>{@link CReferenceTest} for a test of the output using standard finite
value against an
+ * ISO C99 compliant reference implementation.
+ * <li>{@link ComplexEdgeCaseTest} for a test of extreme edge case finite
values for real and/or
+ * imaginary parts that can create intermediate overflow or underflow.
+ * </ul>
+ */
+class ComplexFunctionsTest {
Review Comment:
Is this entire class redundant?
If you update ComplexTest to test all current functions using the Complex
and ComplexFunctions methods with TestUtils.assert methods then you have
already covered all the functionality.
It means that any updated to ComplexTest do not have to be duplicated here.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CStandardTest.java:
##########
@@ -865,12 +884,29 @@ void testImplicitTrig() {
final double re = next(rng);
final double im = next(rng);
final Complex z = complex(re, im);
- final Complex iz = z.multiplyImaginary(1);
- assertComplex(z.asin(), iz.asinh().multiplyImaginary(-1));
- assertComplex(z.atan(), iz.atanh().multiplyImaginary(-1));
- assertComplex(z.cos(), iz.cosh());
- assertComplex(z.sin(), iz.sinh().multiplyImaginary(-1));
- assertComplex(z.tan(), iz.tanh().multiplyImaginary(-1));
+ final Complex iz1 = z.multiplyImaginary(1);
Review Comment:
The code repetition here is prone to error. The operations should be tested
using TestUtils.assert with two operations created using composition:
```Java
ComplexUnaryOperator<ComplexNumber> mi1 = (x, y, out) ->
ComplexFunctions.multiplyImaginary(x, y, 1, out);
ComplexUnaryOperator<ComplexNumber> mim1 = (x, y, out) ->
ComplexFunctions.multiplyImaginary(x, y, -1, out);
UnaryOperator<Complex> asin1 = c ->
c.multiplyImaginary(1).asinh().multiplyImaginary(-1);
ComplexUnaryOperator<ComplexNumber> asin2 =
mi1.andThen(ComplexFunctions::asinh).andThen(mim1);
// Test the two functions produce the same result
// Test it is the expected result
```
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -387,4 +424,314 @@ private static String preprocessTestData(String line,
TestDataFlagOption option,
}
return line;
}
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * No deltas for real or imaginary.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name) {
+ assertComplexScalar(c, operand, operation, expected, actual, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the complex with a scalar operation on the complex number is
equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operand the scalar
+ * @param operation the operation
+ * @param expected the expected
+ * @param actual the actual
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexScalar(Complex c, double operand,
ComplexScalarFunction<ComplexDouble> operation,
+ Complex expected, Complex actual, String
name, double deltaReal, double deltaImaginary) {
+
+ final ComplexDouble result = operation.apply(c, operand,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
actual.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
actual.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the double operation on the complex number is equal to the
expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertDouble(Complex c, DoubleBinaryOperator operation,
+ double expected, String name) {
+ assertDouble(c.getReal(), c.getImaginary(), operation, expected, name,
0.0D);
+ }
+
+ /**
+ * Assert the double operation on the complex number (real and imag parts)
is equal to the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param r real
+ * @param i imaginary
+ * @param operation the operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param delta delta
+ */
+ static void assertDouble(double r, double i, DoubleBinaryOperator
operation,
+ double expected, String name, double delta) {
+
+ final double result = operation.applyAsDouble(r, i);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + expected + "):
result", expected, result, delta);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * No delta.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name) {
+ assertComplexUnary(c, operation1, operation2, expected, name, 0.0D,
0.0D);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param delta delta
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name, double
delta) {
+ assertComplexUnary(c, operation1, operation2, expected, name, delta,
delta);
+ }
+
+ /**
+ * Assert the unary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c the complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexUnary(Complex c,
+ UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexDouble> operation2,
+ Complex expected, String name, double
deltaReal, double deltaImaginary) {
+ final Complex result1 = operation1.apply(c);
+ final ComplexDouble result2 = operation2.apply(c,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c + "." + name + "(): real", expected.real(),
result1.real(), deltaReal);
+ assertEquals(() -> c + "." + name + "(): imaginary", expected.imag(),
result1.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "): real",
expected.real(), result2.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c + "):
imaginary", expected.imag(), result2.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the binary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c1 the first complex
+ * @param c2 the second complex
+ * @param operation1 the operation
+ * @param operation2 the second operation
+ * @param expected the expected
+ * @param name the operation name
+ */
+ static void assertComplexBinary(Complex c1, Complex c2,
+ BinaryOperator<Complex> operation1,
ComplexBinaryOperator<ComplexDouble> operation2,
+ Complex expected, String name) {
+ assertComplexBinary(c1, c2, operation1, operation2, expected, name,
0.0D, 0.0D);
+ }
+
+ /**
+ * Assert the binary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c1 the first complex
+ * @param c2 the second complex
+ * @param operation1 the complex operation
+ * @param operation2 the complexFunctions operation
+ * @param expected the expected result
+ * @param name the operation name
+ * @param deltaReal real delta
+ * @param deltaImaginary imaginary delta
+ */
+ static void assertComplexBinary(Complex c1, Complex c2,
+ BinaryOperator<Complex> operation1,
ComplexBinaryOperator<ComplexDouble> operation2,
+ Complex expected, String name, double
deltaReal, double deltaImaginary) {
+ final Complex result1 = operation1.apply(c1, c2);
+ final ComplexDouble result2 = operation2.apply(c1, c2,
TestUtils.ComplexDoubleConstructor.of());
+
+ assertEquals(() -> c1 + "." + name + "(" + c2 + "): real",
expected.real(), result1.real(), deltaReal);
+ assertEquals(() -> c1 + "." + name + "(" + c2 + "): imaginary",
expected.imag(), result1.imag(), deltaImaginary);
+
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c1 + ", " + c2 +
"): real", expected.real(), result2.getReal(), deltaReal);
+ assertEquals(() -> "ComplexFunctions." + name + "(" + c1 + ", " + c2 +
"): imaginary", expected.imag(), result2.getImaginary(), deltaImaginary);
+ }
+
+ /**
+ * Assert the binary complex operation on the complex number is equal to
the expected value.
+ * If the imaginary part is not NaN the operation must also satisfy the
conjugate equality.
+ *
+ * <pre>
+ * op(conj(z)) = conj(op(z))
+ * </pre>
+ *
+ * <p>The results must be binary equal. This includes the sign of zero.
+ * @param c1 the first complex
+ * @param c2 the second complex
+ * @param operation1 the complex operation
+ * @param operation2 the complexFunctions operation
+ * @param resultChecker function to assert expected result
+ * @param name the operation name
+ */
+ static void assertComplexBinary(Complex c1, Complex c2,
+ BinaryOperator<Complex> operation1,
ComplexBinaryOperator<ComplexDouble> operation2,
+ ComplexConstructor<Boolean> resultChecker,
String name) {
+ final Complex result1 = operation1.apply(c1, c2);
+ final ComplexDouble result2 = operation2.apply(c1, c2,
TestUtils.ComplexDoubleConstructor.of());
+
+ Assertions.assertTrue(resultChecker.apply(result1.getReal(),
result1.getImaginary()), () -> c1 + "." + name + "(" + c2 + ")");
+ Assertions.assertTrue(resultChecker.apply(result2.getReal(),
result2.getImaginary()), () -> "ComplexFunctions." + c1 + "." + name + "(" +
c2 + ")");
+ }
+
+ /**
+ * Assert the two numbers are equal within the provided units of least
precision.
+ * The maximum count of numbers allowed between the two values is {@code
maxUlps - 1}.
+ *
+ * <p>Numbers must have the same sign. Thus -0.0 and 0.0 are never equal.
+ *
+ * @param msg the failure message
+ * @param expected the expected
+ * @param actual the actual
+ * @param delta delta
+ */
+ static void assertEquals(Supplier<String> msg, double expected, double
actual, double delta) {
+ Assertions.assertEquals(expected, actual, delta, msg);
+ }
+
+ static class ComplexDoubleConstructor implements
ComplexConstructor<ComplexDouble>, ComplexDouble {
Review Comment:
This can be simplified:
```Java
static class ComplexNumber {
private double real;
private double imaginary;
ComplexNumber(double real, double imaginary) {
this.real = real;
this.imaginary = imaginary;
}
double getReal() {
return real;
}
double getImaginary() {
return imaginary;
}
}
static void assertEquals(Complex c,
UnaryOperator<Complex> operation1,
ComplexUnaryOperator<ComplexNumber> operation2,
Complex expected, String name, double deltaReal, double
deltaImaginary) {
final Complex result1 = operation1.apply(c);
final ComplexNumber result2 = operation2.apply(c,
ComplexNumber::new);
// ...
}
```
You then test the Complex and ComplexNumber have exactly the same real and
imaginary parts; then the parts are within tolerance of the expected.
The ComplexNumber class is not strictly required. However it does ensure the
methods are decoupled from the Complex class.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.numbers.complex;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class ComplexComposeTest {
Review Comment:
Javadoc would be helpful, e.g. What is the overall purpose of this class?
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CStandardTest.java:
##########
@@ -597,7 +609,9 @@ private static void assertAbs(UniformRandomProvider rng,
for (int i = 0; i < samples; i++) {
double x = fx.applyAsDouble(rng);
double y = fy.applyAsDouble(rng);
- assertAbs(Complex.ofCartesian(x, y), 1);
+ Complex z = Complex.ofCartesian(x, y);
+ assertAbs(z, z.abs(), 1);
Review Comment:
Calling this twice is not required. Just test Complex::abs and
ComplexFunctions::abs are binary equal inside the assertAbs method.
Issue Time Tracking
-------------------
Worklog Id: (was: 790889)
Time Spent: 2.5h (was: 2h 20m)
> Static Method Refactor and Functional Interfaces
> ------------------------------------------------
>
> Key: NUMBERS-188
> URL: https://issues.apache.org/jira/browse/NUMBERS-188
> Project: Commons Numbers
> Issue Type: Sub-task
> Reporter: Sumanth Sulur Rajkumar
> Priority: Minor
> Labels: gsoc2022
> Time Spent: 2.5h
> Remaining Estimate: 0h
>
> Refactored existing instance methods in Complex class as static functions in
> ComplexFunctions class using functional interface signatures
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
