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/06/26 19:30:36 UTC
[GitHub] [commons-numbers] sumanth-rajkumar opened a new pull request, #113: NUMBERS-188: refactored Complex instance methods as static functions
sumanth-rajkumar opened a new pull request, #113:
URL: https://github.com/apache/commons-numbers/pull/113
This PR refactors Complex class using functional interfaces and static methods.
This allows reuse of the refactored static functions for Operations on List/Matrices of Complex Numbers (NUMBERS-186)
The changes retain binary backward compatibility.
Summary of changes
1) Introduced following interfaces for Complex Number Operations
DComplex - interface representing a complex number with double precision in cartesian form
DComplexUnaryOperator - unary operations on DComplex
DComplexBinaryOperator - binary operations on DComplex
DComplexScalarFunction - operations on a DComplex and double type returning a DComplex type
DComplexConstructor - interface to create the DComplex result object from real and imaginary parts
2) Refactored instance methods of Complex class as static functions in ComplexFunctions and ComplexBiFunctions
The static functions use the functional interface signatures described above
3) Added default methods on functional interfaces to compose Complex operations.
--
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
[GitHub] [commons-numbers] sumanth-rajkumar commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar commented on code in PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#discussion_r922429090
##########
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:
Is there a way I can call the ComplexNumber class in the assertComplex operations in the other test classes? Before I said TestUtils.ComplexDoubleConstructor.of() in the apply operation, is there a way to do it like this with your suggested ComplexNumber class
--
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
[GitHub] [commons-numbers] sumanth-rajkumar commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1169322979
Based on feedback on the mailing list, I have made the following changes
1) Rebased and added missing Javadoc comments
2) Removed abstractions for simple functions in the Complex class
3) Fixed line space formatting
4) Renamed the interface DComplex as ComplexDouble
Could you please approve the PR to run the GH workflows?
--
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
[GitHub] [commons-numbers] aherbert closed pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
aherbert closed pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
URL: https://github.com/apache/commons-numbers/pull/113
--
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
[GitHub] [commons-numbers] sumanth-rajkumar commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1167549998
> Currently `complex-gsoc-2022` is up-to-date with `master` so you can rebase on that branch. I have added changes to detect missing javadoc on private methods. Please rebase to collect the changes. Thanks.
--
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
[GitHub] [commons-numbers] codecov-commenter commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1169623756
# [Codecov](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#113](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c293820) into [complex-gsoc-2022](https://codecov.io/gh/apache/commons-numbers/commit/3c446b93903cb94bd25d91ac4e5de1b9971c6d36?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3c446b9) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## complex-gsoc-2022 #113 +/- ##
=======================================================
+ Coverage 99.12% 99.13% +0.01%
- Complexity 1654 1714 +60
=======================================================
Files 64 71 +7
Lines 4227 4302 +75
Branches 835 838 +3
=======================================================
+ Hits 4190 4265 +75
Misses 10 10
Partials 27 27
```
| [Impacted Files](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/commons/numbers/complex/Complex.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXguamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ache/commons/numbers/complex/ComplexFunctions.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhGdW5jdGlvbnMuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...he/commons/numbers/complex/ComplexBiFunctions.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhCaUZ1bmN0aW9ucy5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...commons/numbers/complex/ComplexBinaryOperator.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhCaW5hcnlPcGVyYXRvci5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...he/commons/numbers/complex/ComplexConstructor.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhDb25zdHJ1Y3Rvci5qYXZh) | `100.00% <100.00%> (ø)` | |
| [.../apache/commons/numbers/complex/ComplexDouble.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhEb3VibGUuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...commons/numbers/complex/ComplexScalarFunction.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhTY2FsYXJGdW5jdGlvbi5qYXZh) | `100.00% <100.00%> (ø)` | |
| [.../commons/numbers/complex/ComplexUnaryOperator.java](https://codecov.io/gh/apache/commons-numbers/pull/113/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbXBsZXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21wbGV4L0NvbXBsZXhVbmFyeU9wZXJhdG9yLmphdmE=) | `100.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [3c446b9...c293820](https://codecov.io/gh/apache/commons-numbers/pull/113?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
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.
--
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
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [commons-numbers] sumanth-rajkumar commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar commented on code in PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#discussion_r922429090
##########
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:
Is there a way I can call the ComplexNumber class in the assertComplex operations in the other test classes? Before, I said TestUtils.ComplexDoubleConstructor.of() in the apply operation, is there a way to do it like this with your suggested ComplexNumber class
--
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
[GitHub] [commons-numbers] aherbert commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1167625512
You should add a suppression to:
```
src/main/resources/checkstyle/checkstyle-suppressions.xml
```
There is already this suppression for Complex:
```
<suppress checks="FileLengthCheck" files=".*[/\\]Complex(Test)?\.java" />
```
You may be able to drop the suppression for Complex if it is now under 2000 lines and change it to your other files.
If you rebase on the branch again (because I have now updated the notifications for GH actions to send e-mails upon failure) and I can then approve your PR to allow it to run the GH workflows.
--
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
[GitHub] [commons-numbers] aherbert commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1245216264
This work has been superseded by other PRs merged into the branch to apply the changes in incremental stages.
--
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
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#discussion_r922446529
##########
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:
The class is package private. You should just have to import ` ...TestUtils.ComplexNumber` to use it elsewhere. Or move the ComplexNumber to a new class in the test package to make it clear it is for reuse.
--
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
[GitHub] [commons-numbers] sumanth-rajkumar commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1167559903
Hi Alex,
I had taken out comments of the private methods in ComplexFunctions and ComplexBiFunctions because I had the error of the class being over 2000 lines. Is there a way around this?
--
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
[GitHub] [commons-numbers] sumanth-rajkumar closed pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
sumanth-rajkumar closed pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
URL: https://github.com/apache/commons-numbers/pull/113
--
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
[GitHub] [commons-numbers] aherbert commented on pull request #113: NUMBERS-188: refactored Complex instance methods as static functions
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #113:
URL: https://github.com/apache/commons-numbers/pull/113#issuecomment-1166675879
Currently `complex-gsoc-2022` is up-to-date with `master` so you can rebase on that branch. I have added changes to detect missing javadoc on private methods. Please rebase to collect the changes. Thanks.
--
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