> On 4 Dec 2019, at 19:06, Eric Barnhill <ericbarnh...@gmail.com> wrote: > > NUMBERS-136 was a pretty simple fix, I don't think it will interfere with > anything, so I submitted a PR anyway. Close it if you prefer but I think it > will be easy to integrate. > > Gilles, I saw no way to request you as reviewer, so only requested Alex.
Re: log10: I fixed it already when working on the C99 compliance. Your fix would have worked just as well. However with my fix the method calls Math.log10 which seems to make more sense. I have fixed Complex to have 100% coverage. However although we have 100% coverage a lot of this is from the test that runs the methods with random numbers to check the conjugate equalities. They do not assert the output results. I would like to add a CReferenceTest to go alongside the CStandardTest that tests all the functions using a standard C implementation. Just a test using a standard non-edge case value such as complex(2, 3). This will test the main code path for all functions using non-edge case data. Then perhaps the same again using zero for either real or imaginary as this is a problem case that can occur (see below). When looking at edge cases that are not hit I found a difference between the java implementation and GNU g++. If you do this: ONE.subtract(Complex.ofCartesian(3, 0)) = (-2, 0) But g++ thinks the answer is (-2, -0). The sign of the imaginary component of zero is very important for the output of some functions. I added the previously mentioned subtractFromReal(double minuend). This computes: Complex.ofCartesian(3, 0).subtractFromReal(1) = (-2, -0) The zero is negated and the atanh function now produces the same results as g++. I’ve not made this function public. But it was previously discussed as being part of the C reference. To add, multiply, divide, subtract using real or imaginary only components. For subtract this would be: real - Complex imaginary - Complex Complex - real Complex - imaginary The subtractFromReal definitely has a use case. I will have a look at what comes out of g++ for the entire all-vs-all combination matrix to see if some computations are not possible with the current API. As for C99 compliance I have used g++ as a reference when in doubt. All the functions were previously tested for the listed edge cases with the exception of the cis(y) functionality which I have added. However all the functions (op) must implement the conjugate equality: conj(op(z)) = op(conj(z)) So there are edge case not listed in the references we did not implement. For example: atanh(inf, inf) = 0 + i pi/2 Therefore the conjugate is: atanh(inf, -inf) = 0 + i -pi/2 I added all the cases to the tests for conjugates as well. I also enforced the conjugate equality for all computations when the imaginary is not NaN. So I think I have added too many cases as some are effectively doubled. I will look again tomorrow with fresh eyes and strip redundant tests. I have only looked at the C99 functions. I have not looked at: pow* reciprocal nthRoot There is no reference for these. Although pow is in the reference it doesn’t have edge cases. I can check pow against g++ for all the edge cases. The reciprocal could be tested using 1 / z. So I’ll try that. The only item from the C99 reference I have not tested is the odd / even of the functions, e.g: ‘ctanh is odd’ or ‘ccosh is even’ I think this means: Even: f(z) = f(-z) Odd: f(z) = -f(-z) This relates to reflections of the function. A quick check using g++ shows that this is true. I will look at adding this test tomorrow. So Complex is better than it was. But still needs some work. Alex --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
