You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/07/01 21:19:48 UTC
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
aherbert commented on code in PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#discussion_r912167709
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 apply(ComplexDouble, double).
+ */
+@FunctionalInterface
+public interface ComplexScalarFunction {
+
+ /**
+ * Represents a binary function that accepts a Complex and a double operand to produce a Complex result.
+ * @param c Complex number
+ * @param f factor
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ ComplexDouble apply(ComplexDouble c, double f, ComplexConstructor<ComplexDouble> 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 thenApply(ComplexUnaryOperator after) {
Review Comment:
Composition need not use the ComplexConstructor until the final step:
```Java
return (ComplexDouble c, double f, ComplexConstructor<ComplexDouble> out) ->
after.apply(apply(c, f, Complex::ofCartesian), out);
```
Following `java.util.function.BiFunction` this should be named `andThen`
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexFunctions.java:
##########
@@ -0,0 +1,2631 @@
+/*
+ * 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.function.DoubleUnaryOperator;
+
+/**
+ * Cartesian representation of a complex number. The complex number is expressed
Review Comment:
This javadoc is wrong.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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;
+
+import java.util.Random;
+
+
+class ComplexComposeTest {
+
+
+ private static final ComplexUnaryOperator neg = ComplexFunctions::negate;
+ private static final ComplexUnaryOperator multiplyImag = ComplexFunctions::multiplyImaginary;
+ private static final ComplexUnaryOperator conj = ComplexFunctions::conj;
+ private static final ComplexUnaryOperator multiplyImagConj = multiplyImag.thenApply(conj);
+ private static final ComplexUnaryOperator conjMultiplyImag = conj.thenApply(multiplyImag);
+ private static final ComplexUnaryOperator identity1 = multiplyImagConj.thenApply(multiplyImagConj);
+ private static final ComplexUnaryOperator identity2 = conjMultiplyImag.thenApply(conjMultiplyImag);
+ private static final ComplexBinaryOperator divide = ComplexBiFunctions::divide;
+ private static final ComplexScalarFunction pow = ComplexBiFunctions::pow;
+
+ static class ComplexImpl implements ComplexDouble {
Review Comment:
This class is not required.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexTest.java:
##########
@@ -128,8 +128,8 @@ void testJava() {
@Test
void testCartesianConstructor() {
final Complex z = Complex.ofCartesian(3.0, 4.0);
- Assertions.assertEquals(3.0, z.getReal());
- Assertions.assertEquals(4.0, z.getImaginary());
+ Assertions.assertEquals(3.0, z.real());
Review Comment:
Likewise in this file all the change of getter names adds noise to the PR and is not required.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -455,18 +321,19 @@ public double getReal() {
* <p>This method is the equivalent of the C++ method {@code std::complex::real}.
*
* @return The real part.
- * @see #getReal()
+ * @see #real()
*/
- public double real() {
- return getReal();
+ public double getReal() {
+ return this.real;
Review Comment:
No requirement for `this.`
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexFunctions.java:
##########
@@ -0,0 +1,2631 @@
+/*
+ * 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.function.DoubleUnaryOperator;
+
+/**
+ * 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>.
+ *
+ * <p>This class is all the unary arithmetics. All the arithmetics that uses 1 Complex
+ * number to produce a Complex result.
+ * All arithmetic will create a new instance for the result.</p>
+ */
+public final class ComplexFunctions {
+
+ /**
+ * A complex number representing zero.
+ *
+ * <p>\( (0 + i 0) \).
+ */
+ public static final Complex ZERO = Complex.ZERO;
+
+ /** A complex number representing {@code NaN + i NaN}. */
+ public static final ComplexDouble NAN = Complex.NAN;
Review Comment:
Not required.
This creates an interesting issue where the previous code could consolidate NaN values to a single instance. Now that NaN values must be created with the `ComplexConstructor` there may be multiple instances of Complex.NaN in the JVM.
This highlights a bug in your code where any NaN values are not passed to the complex constructor:
```Java
@Test
void test() {
Complex z = Complex.NAN;
double[] out = {0, 0};
ComplexFunctions.asin(z, (re, im) -> {
out[0] = re;
out[1] = im;
return null;
});
Assertions.assertEquals(Double.NaN, out[0]);
Assertions.assertEquals(Double.NaN, out[1]);
}
```
This highlights that you have not created a test for ComplexFunctions that passes a ComplexConstructor other than Complex::ofCartesian.
It may be wise to update the test suite to ensure that all tests currently applied to Complex are applied to ComplexFunctions using a ComplexConstructor other than Complex, for example using a dummy implementation:
```Java
class ComplexNumber implements ComplexConstructor<ComplexNumber> {
// (r, i) members
@Override
public ComplexNumber apply(double r, double i) {
// store (r, i) ...
return this;
}
}
```
This will detect the edge cases all pass through to the input constructor to create the result. The test should assert the ComplexConstructor received the expected values.
We can discuss options for this on the dev mailing list; ideally we should not have to duplicate too much code in the test suite to target Complex and ComplexFunctions with the same cases.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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;
+
+import java.util.Random;
+
Review Comment:
Please clean up all the use of double blank lines. A single blank line should be used.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexFunctions.java:
##########
@@ -0,0 +1,2631 @@
+/*
+ * 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.function.DoubleUnaryOperator;
+
+/**
+ * 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>.
+ *
+ * <p>This class is all the unary arithmetics. All the arithmetics that uses 1 Complex
+ * number to produce a Complex result.
+ * All arithmetic will create a new instance for the result.</p>
+ */
+public final class ComplexFunctions {
+
+ /**
+ * A complex number representing zero.
+ *
+ * <p>\( (0 + i 0) \).
+ */
+ public static final Complex ZERO = Complex.ZERO;
Review Comment:
Not required.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexConstructor.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;
+
+/**
+ * Define a constructor for a Complex.
+ * @param <R> Generic
+ */
+@FunctionalInterface
+public interface ComplexConstructor<R> {
+
+ /**
+ * A complex result constructor that return an instance of {@link Complex}.
+ */
+ ComplexConstructor<ComplexDouble> D_COMPLEX_RESULT = ComplexDouble::of;
+
+ /**
+ * Represents a function that accepts real and imaginary part of complex number and returns an object.
+ * @param r real part
+ * @param i imaginary part
+ * @return R
+ */
+ R apply(double r, double i);
Review Comment:
Change `r` and `i` to more descriptive names, e.g. `real` and `imaginary`
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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;
+
+import java.util.Random;
+
+
+class ComplexComposeTest {
+
+
+ private static final ComplexUnaryOperator neg = ComplexFunctions::negate;
+ private static final ComplexUnaryOperator multiplyImag = ComplexFunctions::multiplyImaginary;
+ private static final ComplexUnaryOperator conj = ComplexFunctions::conj;
+ private static final ComplexUnaryOperator multiplyImagConj = multiplyImag.thenApply(conj);
+ private static final ComplexUnaryOperator conjMultiplyImag = conj.thenApply(multiplyImag);
+ private static final ComplexUnaryOperator identity1 = multiplyImagConj.thenApply(multiplyImagConj);
+ private static final ComplexUnaryOperator identity2 = conjMultiplyImag.thenApply(conjMultiplyImag);
+ private static final ComplexBinaryOperator divide = ComplexBiFunctions::divide;
+ private static final ComplexScalarFunction pow = ComplexBiFunctions::pow;
+
+ static class ComplexImpl implements ComplexDouble {
+
+ private double real;
+ private double imag;
+
+ ComplexImpl(double r, double i) {
+ real = r;
+ imag = i;
+ }
+
+ @Override
+ public double real() {
+ return 0;
+ }
+
+ @Override
+ public double imag() {
+ return 0;
+ }
+ }
+
+ @Test
+ void testComplexDouble() {
Review Comment:
This test has no assertions. It uses a `ComplexImpl` class which ignores the real and imaginary components.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/ComplexComposeTest.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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;
+
+import java.util.Random;
+
+
+class ComplexComposeTest {
+
+
+ private static final ComplexUnaryOperator neg = ComplexFunctions::negate;
+ private static final ComplexUnaryOperator multiplyImag = ComplexFunctions::multiplyImaginary;
+ private static final ComplexUnaryOperator conj = ComplexFunctions::conj;
+ private static final ComplexUnaryOperator multiplyImagConj = multiplyImag.thenApply(conj);
+ private static final ComplexUnaryOperator conjMultiplyImag = conj.thenApply(multiplyImag);
+ private static final ComplexUnaryOperator identity1 = multiplyImagConj.thenApply(multiplyImagConj);
+ private static final ComplexUnaryOperator identity2 = conjMultiplyImag.thenApply(conjMultiplyImag);
+ private static final ComplexBinaryOperator divide = ComplexBiFunctions::divide;
+ private static final ComplexScalarFunction pow = ComplexBiFunctions::pow;
+
+ static class ComplexImpl implements ComplexDouble {
+
+ private double real;
+ private double imag;
+
+ ComplexImpl(double r, double i) {
+ real = r;
+ imag = i;
+ }
+
+ @Override
+ public double real() {
+ return 0;
+ }
+
+ @Override
+ public double imag() {
+ return 0;
+ }
+ }
+
+ @Test
+ void testComplexDouble() {
+ Random random = new Random();
+ ComplexDouble z1 = new ComplexImpl(random.nextDouble(), random.nextDouble());
+ ComplexDouble z2 = new ComplexImpl(random.nextDouble(), random.nextDouble());
+ ComplexDouble z3 = z1.applyBinaryOperator(z2, divide);
+ ComplexDouble z4 = z1.applyUnaryOperator(neg);
+ ComplexDouble z5 = z1.applyScalarFunction(0, pow);
+ }
+
+ @Test
+ void testUnaryComposing() {
+ Random random = new Random();
+ double real = random.nextInt();
+ double imag = random.nextInt();
+ Complex c = (Complex) ComplexDouble.of(real, imag);
Review Comment:
It is a big assumption to cast the ComplexDouble to Complex. This is done purely to allow the use of `assertEquals`. A better approach is to add an `public static void assertEquals(ComplexDouble, ComplexDouble)` method to TestUtils and only consider the result as a ComplexDouble.
I would prefer to see a composite function tested by declaring the two separate functions and applying them in turn. This can be compared with the result of a composite:
```Java
ComplexUnaryOperator a = ComplexFunctions::multiplyImaginary;
ComplexUnaryOperator b = ComplexFunctions::sqrt;
ComplexUnaryOperator c = a.thenApply(b);
Complex z = Complex.ofCartesian(3, -4);
Assertions.assertEquals(b.apply(a.apply(z)),
c.apply(z));
```
The test can then be parameterized for real and imaginary parts to allow different values including edge cases to be passed through the same test.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexDouble.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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;
+
+/**
+ * Representation of complex number. Contains real and imaginary part and creates Complex
Review Comment:
This can use the same first paragraph as the javadoc for `Complex`.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -1246,55 +860,52 @@ public Complex divideImaginary(double divisor) {
* @see <a href="http://functions.wolfram.com/ElementaryFunctions/Exp/">Exp</a>
*/
public Complex exp() {
- if (Double.isInfinite(real)) {
- // Set the scale factor applied to cis(y)
- double zeroOrInf;
- if (real < 0) {
- if (!Double.isFinite(imaginary)) {
- // (−∞ + i∞) or (−∞ + iNaN) returns (±0 ± i0) (where the signs of the
- // real and imaginary parts of the result are unspecified).
- // Here we preserve the conjugate equality.
- return new Complex(0, Math.copySign(0, imaginary));
- }
- // (−∞ + iy) returns +0 cis(y), for finite y
- zeroOrInf = 0;
- } else {
- // (+∞ + i0) returns +∞ + i0.
- if (imaginary == 0) {
- return this;
- }
- // (+∞ + i∞) or (+∞ + iNaN) returns (±∞ + iNaN) and raises the invalid
- // floating-point exception (where the sign of the real part of the
- // result is unspecified).
- if (!Double.isFinite(imaginary)) {
- return new Complex(real, Double.NaN);
- }
- // (+∞ + iy) returns (+∞ cis(y)), for finite nonzero y.
- zeroOrInf = real;
- }
- return new Complex(zeroOrInf * Math.cos(imaginary),
- zeroOrInf * Math.sin(imaginary));
- } else if (Double.isNaN(real)) {
- // (NaN + i0) returns (NaN + i0)
- // (NaN + iy) returns (NaN + iNaN) and optionally raises the invalid floating-point exception
- // (NaN + iNaN) returns (NaN + iNaN)
- return imaginary == 0 ? this : NAN;
- } else if (!Double.isFinite(imaginary)) {
- // (x + i∞) or (x + iNaN) returns (NaN + iNaN) and raises the invalid
- // floating-point exception, for finite x.
- return NAN;
- }
- // real and imaginary are finite.
- // Compute e^a * (cos(b) + i sin(b)).
-
- // Special case:
- // (±0 + i0) returns (1 + i0)
- final double exp = Math.exp(real);
- if (imaginary == 0) {
- return new Complex(exp, imaginary);
- }
- return new Complex(exp * Math.cos(imaginary),
- exp * Math.sin(imaginary));
+ return this.applyUnaryOperator(ComplexFunctions::exp);
+ }
+
+ /**
+ * Returns the
+ * <a href="http://mathworld.wolfram.com/ComplexConjugate.html">conjugate</a>
+ * \( \overline{z} \) of this complex number \( z \).
+ *
+ * <p>\[ \begin{aligned}
+ * z &= a + i b \\
+ * \overline{z} &= a - i b \end{aligned}\]
+ *
+ * @return The conjugate (\( \overline{z} \)) of this complex number.
+ */
+ public Complex conj() {
+ return new Complex(real, -imaginary);
+ }
+
+ /**
+ * This operator is used for all Complex operations that deals with one Complex number
+ * and multiplies the Complex number by i and then -i.
+ * @param operator ComplexUnaryOperator
+ * @return Complex
+ */
+ private Complex multiplyIApplyAndThenMultiplyNegativeI(ComplexUnaryOperator operator) {
Review Comment:
This function should be redundant. At all the call sites there should be a call to ComplexFunctions to create the required result, e.g there should be a `atan` function as well as a `atanh` function in ComplexFunctions.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 apply(ComplexDouble, double).
+ */
+@FunctionalInterface
+public interface ComplexScalarFunction {
+
+ /**
+ * Represents a binary function that accepts a Complex and a double operand to produce a Complex result.
+ * @param c Complex number
+ * @param f factor
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ ComplexDouble apply(ComplexDouble c, double f, ComplexConstructor<ComplexDouble> 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 thenApply(ComplexUnaryOperator after) {
+ Objects.requireNonNull(after);
+ return (ComplexDouble c, double f, ComplexConstructor<ComplexDouble> out) -> after.apply(apply(c, f, out), out);
+
Review Comment:
Please check the code for these redundant additional whitespace lines.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexFunctions.java:
##########
@@ -0,0 +1,2631 @@
+/*
+ * 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.function.DoubleUnaryOperator;
+
+/**
+ * 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>.
+ *
+ * <p>This class is all the unary arithmetics. All the arithmetics that uses 1 Complex
+ * number to produce a Complex result.
+ * All arithmetic will create a new instance for the result.</p>
+ */
+public final class ComplexFunctions {
Review Comment:
I think this class should use the same method order as the original `Complex` class for unary and binary operators, essentially just moving the entire code in here with methods taking a ComplexConstructor.
Although the Complex class implements the simple unary operators (e.g. conj, negate) for efficiency these should be duplicated in this class too. This class should represent the entire public API of Complex but with the output result specified with the ComplexConstructor.
The ComplexBiFunctions class is not required.
Note: If the functional interfaces are updated to specify the functional methods as `apply(double re, double im, ...)` then this class can be changed to expose all the private methods that operate on primitive arguments to public. This will make the class more useable by operating on either a decomposed complex using primitive or an instance of a complex type. This should be discussed on the dev mailing list.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 apply(ComplexDouble, double).
+ */
+@FunctionalInterface
+public interface ComplexScalarFunction {
+
+ /**
+ * Represents a binary function that accepts a Complex and a double operand to produce a Complex result.
+ * @param c Complex number
+ * @param f factor
Review Comment:
It may not always be a factor. Change the names to use the JDK BiFunction terminology:
```Java
* @param t the first function argument
* @param u the second function argument
```
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -646,21 +483,6 @@ public boolean isFinite() {
return Double.isFinite(real) && Double.isFinite(imaginary);
Review Comment:
I think the logic for isNaN, isInfinite and isFinite should be public in ComplexFunctions.
This would fix javadoc errors in ComplexFunctions where the `{@link #isInfintite(ComplexDouble)}` tags are errors.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -1246,55 +860,52 @@ public Complex divideImaginary(double divisor) {
* @see <a href="http://functions.wolfram.com/ElementaryFunctions/Exp/">Exp</a>
*/
public Complex exp() {
- if (Double.isInfinite(real)) {
- // Set the scale factor applied to cis(y)
- double zeroOrInf;
- if (real < 0) {
- if (!Double.isFinite(imaginary)) {
- // (−∞ + i∞) or (−∞ + iNaN) returns (±0 ± i0) (where the signs of the
- // real and imaginary parts of the result are unspecified).
- // Here we preserve the conjugate equality.
- return new Complex(0, Math.copySign(0, imaginary));
- }
- // (−∞ + iy) returns +0 cis(y), for finite y
- zeroOrInf = 0;
- } else {
- // (+∞ + i0) returns +∞ + i0.
- if (imaginary == 0) {
- return this;
- }
- // (+∞ + i∞) or (+∞ + iNaN) returns (±∞ + iNaN) and raises the invalid
- // floating-point exception (where the sign of the real part of the
- // result is unspecified).
- if (!Double.isFinite(imaginary)) {
- return new Complex(real, Double.NaN);
- }
- // (+∞ + iy) returns (+∞ cis(y)), for finite nonzero y.
- zeroOrInf = real;
- }
- return new Complex(zeroOrInf * Math.cos(imaginary),
- zeroOrInf * Math.sin(imaginary));
- } else if (Double.isNaN(real)) {
- // (NaN + i0) returns (NaN + i0)
- // (NaN + iy) returns (NaN + iNaN) and optionally raises the invalid floating-point exception
- // (NaN + iNaN) returns (NaN + iNaN)
- return imaginary == 0 ? this : NAN;
- } else if (!Double.isFinite(imaginary)) {
- // (x + i∞) or (x + iNaN) returns (NaN + iNaN) and raises the invalid
- // floating-point exception, for finite x.
- return NAN;
- }
- // real and imaginary are finite.
- // Compute e^a * (cos(b) + i sin(b)).
-
- // Special case:
- // (±0 + i0) returns (1 + i0)
- final double exp = Math.exp(real);
- if (imaginary == 0) {
- return new Complex(exp, imaginary);
- }
- return new Complex(exp * Math.cos(imaginary),
- exp * Math.sin(imaginary));
+ return this.applyUnaryOperator(ComplexFunctions::exp);
+ }
+
+ /**
+ * Returns the
+ * <a href="http://mathworld.wolfram.com/ComplexConjugate.html">conjugate</a>
+ * \( \overline{z} \) of this complex number \( z \).
+ *
+ * <p>\[ \begin{aligned}
+ * z &= a + i b \\
+ * \overline{z} &= a - i b \end{aligned}\]
+ *
+ * @return The conjugate (\( \overline{z} \)) of this complex number.
+ */
+ public Complex conj() {
+ return new Complex(real, -imaginary);
+ }
+
+ /**
+ * This operator is used for all Complex operations that deals with one Complex number
+ * and multiplies the Complex number by i and then -i.
+ * @param operator ComplexUnaryOperator
+ * @return Complex
+ */
+ private Complex multiplyIApplyAndThenMultiplyNegativeI(ComplexUnaryOperator operator) {
+ return (Complex) operator.apply(-this.imaginary, this.real, Complex::multiplyNegativeI);
+ }
+
+ /**
+ * This operator is used for all Complex operations that deals with one Complex number
+ * and multiplies the Complex number by i.
+ * @param operator ComplexUnaryOperator
+ * @return Complex
+ */
+ private Complex multiplyIAndApply(ComplexUnaryOperator operator) {
Review Comment:
This should also be redundant
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -445,7 +310,8 @@ private static String parsingExceptionMsg(String message,
*
* @return The real part.
*/
- public double getReal() {
+ @Override
Review Comment:
Why have you reordered the real and getReal methods? Please try to create minimal diffs.
Note this has introduced a javadoc error in e.g. `getReal()` which states it is the c++ equivalent of `std::complex::real` whereas that comment applies to `real()`
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -3285,43 +1712,36 @@ 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;
+ @Override
+ public Complex applyUnaryOperator(ComplexUnaryOperator operator) {
Review Comment:
I do not think these should be public. These functions are inherited from the ComplexDouble interface. I do not think they should be present in that interface.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 apply(ComplexDouble, double).
Review Comment:
The functional method is `apply(ComplexDouble, double, ComplexConstructor<ComplexDouble>)`. This should be referenced using a `{@link ...}` javadoc tag.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/Complex.java:
##########
@@ -82,128 +83,7 @@ public final class Complex implements Serializable {
public static final Complex ZERO = new Complex(0, 0);
/** A complex number representing {@code NaN + i NaN}. */
- private static final Complex NAN = new Complex(Double.NaN, Double.NaN);
- /** π/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);
- /** 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 1/2}. */
- private static final double HALF = 0.5;
- /** {@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;
- /** The bit representation of {@code -0.0}. */
- private static final long NEGATIVE_ZERO_LONG_BITS = Double.doubleToLongBits(-0.0);
- /** Exponent offset in IEEE754 representation. */
- private static final int EXPONENT_OFFSET = 1023;
- /**
- * 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);
- /** Mask to remove the sign bit from a long. */
- private static final long UNSIGN_MASK = 0x7fff_ffff_ffff_ffffL;
- /** Mask to extract the 52-bit mantissa from a long representation of a double. */
- private static final long MANTISSA_MASK = 0x000f_ffff_ffff_ffffL;
- /** 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;
+ public static final Complex NAN = new Complex(Double.NaN, Double.NaN);
Review Comment:
Why is this public? This member is now redundant.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexDouble.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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;
+
+/**
+ * Representation of complex number. Contains real and imaginary part and creates Complex
+ * number using real and imaginary part.
+ */
+public interface ComplexDouble {
+
+ /**
+ * Gets the real part \( a \) of complex number \( (a + i b) \).
+ *
+ * @return real part
+ */
+ double real();
+
+ /**
+ * Gets the imaginary part \( b \) of complex number \( (a + i b) \).
+ *
+ * @return imaginary part
+ */
+ double imag();
+
+ /**
+ * Create a complex number given the real and imaginary parts.
+ *
+ * @param r Real part.
+ * @param i Imaginary part.
+ * @return {@code ComplexDouble} number.
+ */
+ static ComplexDouble of(double r, double i) {
+ return Complex.ofCartesian(r, i);
+ }
+
+ /**
+ * This operator is used for all Complex operations that only deal with one Complex number.
+ * @param operator ComplexUnaryOperator
+ * @return ComplexDouble
+ */
+ default ComplexDouble applyUnaryOperator(ComplexUnaryOperator operator) {
Review Comment:
These default methods to apply an operator and return a `ComplexDouble` with a hard coded choice of return type to Complex; this should be an implementor's choice. I do not see why these methods are required.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexDouble.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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;
+
+/**
+ * Representation of complex number. Contains real and imaginary part and creates Complex
+ * number using real and imaginary part.
+ */
+public interface ComplexDouble {
+
+ /**
+ * Gets the real part \( a \) of complex number \( (a + i b) \).
+ *
+ * @return real part
+ */
+ double real();
+
+ /**
+ * Gets the imaginary part \( b \) of complex number \( (a + i b) \).
+ *
+ * @return imaginary part
+ */
+ double imag();
+
+ /**
+ * Create a complex number given the real and imaginary parts.
+ *
+ * @param r Real part.
+ * @param i Imaginary part.
+ * @return {@code ComplexDouble} number.
+ */
+ static ComplexDouble of(double r, double i) {
Review Comment:
This method does not belong in this interface. The choice of the implementation should not be hard coded as a Complex.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/TestUtils.java:
##########
@@ -63,8 +63,8 @@ private TestUtils() {
* @param actual the actual value
*/
public static void assertSame(Complex expected, Complex actual) {
- Assertions.assertEquals(expected.getReal(), actual.getReal());
- Assertions.assertEquals(expected.getImaginary(), actual.getImaginary());
+ Assertions.assertEquals(expected.real(), actual.real());
Review Comment:
Likewise in this file all the change of getter names adds noise to the PR and is not required.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexScalarFunction.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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 apply(ComplexDouble, double).
+ */
+@FunctionalInterface
+public interface ComplexScalarFunction {
+
+ /**
+ * Represents a binary function that accepts a Complex and a double operand to produce a Complex result.
+ * @param c Complex number
+ * @param f factor
+ * @param result Constructor
+ * @return ComplexDouble
+ */
+ ComplexDouble apply(ComplexDouble c, double f, ComplexConstructor<ComplexDouble> result);
Review Comment:
This interface should be typed: the result is accepted by the ComplexConstructor and this can be typed.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.UnaryOperator;
+
+/**
+ * Represents a complex operation that accepts a complex number of type ComplexDouble and produces a ComplexDouble result.
+ */
+@FunctionalInterface
+public interface ComplexUnaryOperator extends UnaryOperator<ComplexDouble> {
+
+
+ /**
+ * 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
+ */
+ ComplexDouble apply(ComplexDouble in, ComplexConstructor<ComplexDouble> 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
+ */
+ default ComplexDouble apply(double r, double i, ComplexConstructor<ComplexDouble> out) {
Review Comment:
Note that by making this a default that creates an instance of `Complex` you impose memory allocation overhead to any call site that just has the real and imaginary parts (e.g. a structure storing a list of complex numbers using primitive arrays).
The `ComplexDouble apply(ComplexDouble in, ComplexConstructor<ComplexDouble> out)` method should be a default that calls this function using the real and imaginary parts.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexBiFunctions.java:
##########
@@ -0,0 +1,494 @@
+/*
+ * 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
Review Comment:
This javadoc is wrong.
This class can be merged with ComplexFunctions. I do not see why a separate utility class is required.
##########
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.UnaryOperator;
+
+/**
+ * Represents a complex operation that accepts a complex number of type ComplexDouble and produces a ComplexDouble result.
+ */
+@FunctionalInterface
+public interface ComplexUnaryOperator extends UnaryOperator<ComplexDouble> {
+
+
+ /**
+ * 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
+ */
+ ComplexDouble apply(ComplexDouble in, ComplexConstructor<ComplexDouble> 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
+ */
+ default ComplexDouble apply(double r, double i, ComplexConstructor<ComplexDouble> out) {
+ return apply(Complex.ofCartesian(r, i), out);
+ }
+
+ /**
+ * Represents an operator that accepts a complex number and produces a result.
+ * @param c Complex number
+ * @return ComplexDouble
+ */
+ @Override
+ default ComplexDouble apply(ComplexDouble c) {
+ return apply(c, ComplexConstructor.D_COMPLEX_RESULT);
+ }
+
+ /**
+ * 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 thenApply(ComplexUnaryOperator after) {
Review Comment:
Following `java.util.function.Function` this should have a `compose` and a `andThen`.
There is no precedent in the JDK functions for composites that expand the argument list (making a unary operator into a binary operator), i.e. the methods `thenApplyBinaryOperator` and `thenApplyScalarFunction`. Can these be removed?
I note you have used them in the pow functions. But these are not required in the public API.
##########
commons-numbers-complex/src/test/java/org/apache/commons/numbers/complex/CStandardTest.java:
##########
@@ -137,14 +137,14 @@ private enum UnspecifiedSign {
REAL {
@Override
Complex removeSign(Complex c) {
- return negative(c.getReal()) ? complex(-c.getReal(), c.getImaginary()) : c;
+ return negative(c.real()) ? complex(-c.real(), c.imag()) : c;
Review Comment:
It is unclear why you have changed all these getters.
It is Java convention for a getter to be `getX`. The `real` and `imag` methods are convenience methods for C developers to use the same familiar syntax.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org