You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Alex Herbert <al...@gmail.com> on 2019/11/06 17:51:13 UTC
[numbers] Bug in complex multiply + divide + isNaN
The method Complex::multiply(Complex) has the following check to recover
infinities:
double x = ac - bd;
double y = ad + bc;
if (Double.isNaN(a) && Double.isNaN(b)) {
The divide method checks x and y:
double x = Math.scalb( (a*c + b*d) / denom, -ilogbw);
double y = Math.scalb( (b*c - a*d) / denom, -ilogbw);
if (Double.isNaN(x) && Double.isNaN(y)) {
So the multiply method should be checking if x and y are NaN.
The multiply method also has this apparent cut-and-paste error:
a = Math.copySign(Double.isInfinite(a) ? 1.0 : 0.0, a);
b = Math.copySign(Double.isInfinite(a) ? 1.0 : 0.0, a);
where b should be assigned the sign from b not a.
I tried to find the C.99 standard for this as mentioned in the javadoc
and came up with this [1]. The implementation of multiply is provided on
page 470 and matches that in Complex without the afore mentioned bugs.
The definition for divide suggests switching from
!Double.isInfinite
to
Double.isFinite
which will return false for values that are NaN (where as
!Double.isInfinite will be true for NaN leading to an incorrect choice
in the algorithm).
The second correction statement is wrong:
} else if ((Double.isInfinite(a) && Double.isInfinite(b)) &&
!Double.isInfinite(c) && !Double.isInfinite(d)) {
should be:
} else if ((Double.isInfinite(a) || Double.isInfinite(b)) &&
!Double.isInfinite(c) && !Double.isInfinite(d)) {
The rest of divide is correct.
About isNaN:
"A complex or imaginary value with at least one infinite part is
regarded as an infinity
(even if its other part is a NaN). A complex or imaginary value is a
finite number if each
of its parts is a finite number (neither infinite nor NaN). A complex or
imaginary value is
a zero if each of its parts is a zero."
So Complex::isInfinite is defined correctly but Complex::isNaN does not
check if components are infinite, only if one of them is NaN. This means
you can be both infinite and NaN which does not seem correct.
This suggests updating isNaN to:
public boolean isNaN() {
return Double.isNaN(real) &&
Double.isNaN(imaginary);
}
I do not know how this will effect any computations that use isNaN().
The C.99 standard I found was a draft but the main host site states that
the working paper is the consolidated standard [2].
Any objections to updating multiply/divide/isNaN to match the standard?
I'll add unit tests to hit the edge cases that should fail with the
current implementation.
[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
[2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
Hi.
Le jeu. 7 nov. 2019 à 01:48, Alex Herbert <al...@gmail.com> a écrit :
>
>
>
> > On 7 Nov 2019, at 00:34, Gilles Sadowski <gi...@gmail.com> wrote:
> >
> > 2019-11-07 1:03 UTC+01:00, Alex Herbert <alex.d.herbert@gmail.com <ma...@gmail.com>>:
> >>
> >>
> >>> On 6 Nov 2019, at 23:17, Eric Barnhill <ericbarnhill@gmail.com <ma...@gmail.com>> wrote:
> >>>
> >>> +1 on all suggestions. Thanks, Alex.
> >>
> >> I’ve borrowed the checkstyle, PMD and spot bugs config from commons-rng. I
> >> updated the parent to 49 and updated the travis build script to run the
> >> check goal instead of creating reports.
> >
> > I had updated the PMD configuration (copying from [RNG] and removing
> > references to specific classes) this afternoon.
>
> I did not really change the PMD config. Just a bump of the PMD versions.
>
> However pmd:check was never run so this is now in the default goal and run on travis.
I used to review this after
$ mvn site site:stage
> Travis currently has a problem with the length of the log file being too long as Maven downloads the entire internet. So I’m trying a fix for that.
>
> >
> >>
> >> This requires disabling checkstyle:check and pmd:check from failing.
> >> Checkstyle errors were previously listed. PMD has the following errors:
> >>
> >> [INFO] You have 23 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-core/target/pmd.xml
> >> [INFO] You have 16 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-complex/target/pmd.xml
> >> [INFO] You have 30 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/target/pmd.xml
> >> [INFO] You have 30 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-primes/target/pmd.xml
> >> [INFO] You have 13 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/target/pmd.xml
> >> [INFO] You have 42 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-fraction/target/pmd.xml
> >> [INFO] You have 3 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-angle/target/pmd.xml
> >> [INFO] You have 192 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-gamma/target/pmd.xml
> >> [INFO] You have 35 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/target/pmd.xml
> >> [INFO] You have 14 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-arrays/target/pmd.xml
> >> [INFO] You have 10 PMD violations. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-field/target/pmd.xml
> >> [INFO] You have 1 PMD violation. For more details see:
> >> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/target/pmd.xml
> >
> > Yes, those should be fixed (and perhaps some could be considered
> > as false positives).
>
> I’m not planning on doing a lot of fixing. With RNG I made most of the PMD issues go away with some rule tweaking.
Quite fine; many of these "violations" tends to be nits (like "Do not
reuse method
arguments") and should not be blocking a release; the issue were in Commons Math
forever without anyone bothering.
>
> I will fix checkstyle though. This can be semi automated.
I can help fixing those, once you've pushed you pending changes to "master".
Gilles
> >> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 7 Nov 2019, at 00:34, Gilles Sadowski <gi...@gmail.com> wrote:
>
> 2019-11-07 1:03 UTC+01:00, Alex Herbert <alex.d.herbert@gmail.com <ma...@gmail.com>>:
>>
>>
>>> On 6 Nov 2019, at 23:17, Eric Barnhill <ericbarnhill@gmail.com <ma...@gmail.com>> wrote:
>>>
>>> +1 on all suggestions. Thanks, Alex.
>>
>> I’ve borrowed the checkstyle, PMD and spot bugs config from commons-rng. I
>> updated the parent to 49 and updated the travis build script to run the
>> check goal instead of creating reports.
>
> I had updated the PMD configuration (copying from [RNG] and removing
> references to specific classes) this afternoon.
I did not really change the PMD config. Just a bump of the PMD versions.
However pmd:check was never run so this is now in the default goal and run on travis.
Travis currently has a problem with the length of the log file being too long as Maven downloads the entire internet. So I’m trying a fix for that.
>
>>
>> This requires disabling checkstyle:check and pmd:check from failing.
>> Checkstyle errors were previously listed. PMD has the following errors:
>>
>> [INFO] You have 23 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-core/target/pmd.xml
>> [INFO] You have 16 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/target/pmd.xml
>> [INFO] You have 30 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/target/pmd.xml
>> [INFO] You have 30 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/target/pmd.xml
>> [INFO] You have 13 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/target/pmd.xml
>> [INFO] You have 42 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/target/pmd.xml
>> [INFO] You have 3 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/target/pmd.xml
>> [INFO] You have 192 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/target/pmd.xml
>> [INFO] You have 35 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/target/pmd.xml
>> [INFO] You have 14 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/target/pmd.xml
>> [INFO] You have 10 PMD violations. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-field/target/pmd.xml
>> [INFO] You have 1 PMD violation. For more details see:
>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/target/pmd.xml
>
> Yes, those should be fixed (and perhaps some could be considered
> as false positives).
I’m not planning on doing a lot of fixing. With RNG I made most of the PMD issues go away with some rule tweaking.
I will fix checkstyle though. This can be semi automated.
>
>>
>> A simple fix for apache-rat:check and spotbugs:check was needed.
>>
>> Javadoc:javadoc builds on JDK 11 which is strict so that is good.
>>
>> All the changes are in a PR which passes 'mvn test site’. If travis is OK
>> I’ll merge to master and then set to fixing checkstyle and PMD.
>>
>> Then I’ll fix Complex.
>
> OK.
>
> Regards,
> Gilles
>
>>
>>>
>>> On Wed, Nov 6, 2019 at 2:38 PM Alex Herbert <al...@gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>>>>>
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>> Any objections to updating multiply/divide/isNaN to match the
>>>>>> standard?
>>>>>
>>>>> Let me think... ;-)
>>>>
>>>> OK, I’ll fix it and double check the other tests against the c
>>>> reference.
>>>>
>>>>>
>>>>>>
>>>>>> I'll add unit tests to hit the edge cases that should fail with the
>>>>>> current implementation.
>>>>>
>>>>> Thanks,
>>>>> Gilles
>>>>
>>>> Are changes to numbers going under Jira tickets?
>>>>
>>>> It looks like it needs an update to checkstyle, PMD, spotbugs, the
>>>> commons-parent and travis.
>>>>
>>>> Checkstyle config from commons-rng finds:
>>>>
>>>> [INFO] There are 115 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 202 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 102 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 276 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 68 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 289 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 56 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 50 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>>
>>>> The mass of errors is white space style in the test classes. Without the
>>>> test classes the result is:
>>>>
>>>> [INFO] There are 12 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 54 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 49 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 5 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 3 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 20 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>>>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>>>> ruleset.
>>>>
>>>>
>>>> Also looking at Complex it would benefit from:
>>>>
>>>> public Complex subtractFrom(double minuend) {
>>>> return new Complex(minuend - real, imaginary);
>>>> }
>>>>
>>>> This would avoid:
>>>>
>>>> Complex a = …;
>>>> double b = …;
>>>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>>>> Complex d = Complex.ofReal(b).subtract(a).conj();
>>>>
>>>> With
>>>>
>>>> Complex a = …;
>>>> double b = …;
>>>> Complex c = a.subtractFrom(b);
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>>>>>
>>>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>
>>>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org <ma...@commons.apache.org>
> For additional commands, e-mail: dev-help@commons.apache.org <ma...@commons.apache.org>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
2019-11-07 1:03 UTC+01:00, Alex Herbert <al...@gmail.com>:
>
>
>> On 6 Nov 2019, at 23:17, Eric Barnhill <er...@gmail.com> wrote:
>>
>> +1 on all suggestions. Thanks, Alex.
>
> I’ve borrowed the checkstyle, PMD and spot bugs config from commons-rng. I
> updated the parent to 49 and updated the travis build script to run the
> check goal instead of creating reports.
I had updated the PMD configuration (copying from [RNG] and removing
references to specific classes) this afternoon.
>
> This requires disabling checkstyle:check and pmd:check from failing.
> Checkstyle errors were previously listed. PMD has the following errors:
>
> [INFO] You have 23 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-core/target/pmd.xml
> [INFO] You have 16 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-complex/target/pmd.xml
> [INFO] You have 30 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/target/pmd.xml
> [INFO] You have 30 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-primes/target/pmd.xml
> [INFO] You have 13 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/target/pmd.xml
> [INFO] You have 42 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-fraction/target/pmd.xml
> [INFO] You have 3 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-angle/target/pmd.xml
> [INFO] You have 192 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-gamma/target/pmd.xml
> [INFO] You have 35 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/target/pmd.xml
> [INFO] You have 14 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-arrays/target/pmd.xml
> [INFO] You have 10 PMD violations. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-field/target/pmd.xml
> [INFO] You have 1 PMD violation. For more details see:
> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/target/pmd.xml
Yes, those should be fixed (and perhaps some could be considered
as false positives).
>
> A simple fix for apache-rat:check and spotbugs:check was needed.
>
> Javadoc:javadoc builds on JDK 11 which is strict so that is good.
>
> All the changes are in a PR which passes 'mvn test site’. If travis is OK
> I’ll merge to master and then set to fixing checkstyle and PMD.
>
> Then I’ll fix Complex.
OK.
Regards,
Gilles
>
>>
>> On Wed, Nov 6, 2019 at 2:38 PM Alex Herbert <al...@gmail.com>
>> wrote:
>>
>>>
>>>
>>>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>>>>
>>>>> [...]
>>>>>
>>>>>
>>>>> Any objections to updating multiply/divide/isNaN to match the
>>>>> standard?
>>>>
>>>> Let me think... ;-)
>>>
>>> OK, I’ll fix it and double check the other tests against the c
>>> reference.
>>>
>>>>
>>>>>
>>>>> I'll add unit tests to hit the edge cases that should fail with the
>>>>> current implementation.
>>>>
>>>> Thanks,
>>>> Gilles
>>>
>>> Are changes to numbers going under Jira tickets?
>>>
>>> It looks like it needs an update to checkstyle, PMD, spotbugs, the
>>> commons-parent and travis.
>>>
>>> Checkstyle config from commons-rng finds:
>>>
>>> [INFO] There are 115 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 202 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 102 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 276 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 68 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 289 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 56 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 50 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>>
>>> The mass of errors is white space style in the test classes. Without the
>>> test classes the result is:
>>>
>>> [INFO] There are 12 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 54 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 49 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 5 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 3 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 20 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>>> ruleset.
>>>
>>>
>>> Also looking at Complex it would benefit from:
>>>
>>> public Complex subtractFrom(double minuend) {
>>> return new Complex(minuend - real, imaginary);
>>> }
>>>
>>> This would avoid:
>>>
>>> Complex a = …;
>>> double b = …;
>>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>>> Complex d = Complex.ofReal(b).subtract(a).conj();
>>>
>>> With
>>>
>>> Complex a = …;
>>> double b = …;
>>> Complex c = a.subtractFrom(b);
>>>
>>>
>>>>
>>>>>
>>>>>
>>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>>>>
>>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>
>>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 6 Nov 2019, at 23:17, Eric Barnhill <er...@gmail.com> wrote:
>
> +1 on all suggestions. Thanks, Alex.
I’ve borrowed the checkstyle, PMD and spot bugs config from commons-rng. I updated the parent to 49 and updated the travis build script to run the check goal instead of creating reports.
This requires disabling checkstyle:check and pmd:check from failing. Checkstyle errors were previously listed. PMD has the following errors:
[INFO] You have 23 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-core/target/pmd.xml
[INFO] You have 16 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-complex/target/pmd.xml
[INFO] You have 30 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/target/pmd.xml
[INFO] You have 30 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-primes/target/pmd.xml
[INFO] You have 13 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-quaternion/target/pmd.xml
[INFO] You have 42 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-fraction/target/pmd.xml
[INFO] You have 3 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-angle/target/pmd.xml
[INFO] You have 192 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-gamma/target/pmd.xml
[INFO] You have 35 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/target/pmd.xml
[INFO] You have 14 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-arrays/target/pmd.xml
[INFO] You have 10 PMD violations. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-field/target/pmd.xml
[INFO] You have 1 PMD violation. For more details see: /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/target/pmd.xml
A simple fix for apache-rat:check and spotbugs:check was needed.
Javadoc:javadoc builds on JDK 11 which is strict so that is good.
All the changes are in a PR which passes 'mvn test site’. If travis is OK I’ll merge to master and then set to fixing checkstyle and PMD.
Then I’ll fix Complex.
>
> On Wed, Nov 6, 2019 at 2:38 PM Alex Herbert <al...@gmail.com>
> wrote:
>
>>
>>
>>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>>>
>>>> [...]
>>>>
>>>>
>>>> Any objections to updating multiply/divide/isNaN to match the standard?
>>>
>>> Let me think... ;-)
>>
>> OK, I’ll fix it and double check the other tests against the c reference.
>>
>>>
>>>>
>>>> I'll add unit tests to hit the edge cases that should fail with the
>>>> current implementation.
>>>
>>> Thanks,
>>> Gilles
>>
>> Are changes to numbers going under Jira tickets?
>>
>> It looks like it needs an update to checkstyle, PMD, spotbugs, the
>> commons-parent and travis.
>>
>> Checkstyle config from commons-rng finds:
>>
>> [INFO] There are 115 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 202 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 102 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 276 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 68 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 289 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 56 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 50 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>>
>> The mass of errors is white space style in the test classes. Without the
>> test classes the result is:
>>
>> [INFO] There are 12 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 54 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 49 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 5 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 20 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>>
>>
>> Also looking at Complex it would benefit from:
>>
>> public Complex subtractFrom(double minuend) {
>> return new Complex(minuend - real, imaginary);
>> }
>>
>> This would avoid:
>>
>> Complex a = …;
>> double b = …;
>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>> Complex d = Complex.ofReal(b).subtract(a).conj();
>>
>> With
>>
>> Complex a = …;
>> double b = …;
>> Complex c = a.subtractFrom(b);
>>
>>
>>>
>>>>
>>>>
>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>>>
>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Eric Barnhill <er...@gmail.com>.
+1 on all suggestions. Thanks, Alex.
On Wed, Nov 6, 2019 at 2:38 PM Alex Herbert <al...@gmail.com>
wrote:
>
>
> > On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
> >
> >> [...]
> >>
> >>
> >> Any objections to updating multiply/divide/isNaN to match the standard?
> >
> > Let me think... ;-)
>
> OK, I’ll fix it and double check the other tests against the c reference.
>
> >
> >>
> >> I'll add unit tests to hit the edge cases that should fail with the
> >> current implementation.
> >
> > Thanks,
> > Gilles
>
> Are changes to numbers going under Jira tickets?
>
> It looks like it needs an update to checkstyle, PMD, spotbugs, the
> commons-parent and travis.
>
> Checkstyle config from commons-rng finds:
>
> [INFO] There are 115 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 202 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 102 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 276 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 68 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 289 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 10 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 56 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 50 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 10 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 4 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
>
> The mass of errors is white space style in the test classes. Without the
> test classes the result is:
>
> [INFO] There are 12 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 54 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 19 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 49 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 5 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 6 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 3 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 20 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 19 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 4 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 6 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
>
>
> Also looking at Complex it would benefit from:
>
> public Complex subtractFrom(double minuend) {
> return new Complex(minuend - real, imaginary);
> }
>
> This would avoid:
>
> Complex a = …;
> double b = …;
> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
> Complex d = Complex.ofReal(b).subtract(a).conj();
>
> With
>
> Complex a = …;
> double b = …;
> Complex c = a.subtractFrom(b);
>
>
> >
> >>
> >>
> >> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
> >>
> >> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 7 Nov 2019, at 17:31, Eric Barnhill <er...@gmail.com> wrote:
>
> […]
>
> Well that's interesting, I did not see that the standard specified
> all-vs-all methods in all those cases. There isn't a performance gain for
> multiplying by an imaginary double like there is for a real double,
Not for imaginary against real, but the matrix states there is for imaginary against complex:
Multiplying by a real double:
u * (x + iy) = (xu) + i (yu)
Multiplying by an imaginary (no real):
iv * (x + iy) = (-yv) + i (xv)
So the performance gain is the same. One use case is in the previously mentioned trigonomic identities where you wish to multiply by I or -I. In this case v is 1 or -1 and so the specialisation is:
i * (x + iy) = -y + ix
-i * (x + iy) = y + i-x
I’ve used this specialisation internally but I wonder if it would be useful in the API.
Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 7 Nov 2019, at 17:31, Eric Barnhill <er...@gmail.com> wrote:
>
> On Thu, Nov 7, 2019 at 3:59 AM Alex Herbert <al...@gmail.com>
> wrote:
>
>>
>> There is a matrix for real/imaginary/complex all-vs-all additive and
>> multiplicative operators in the standard (tables in G.5.1 and G.5.2).
>> The question is do we want to support the entire matrix:
>>
>> Covered:
>>
>> Complex.multiplyReal(double x) as Complex.multiply(double x)
>> Complex.divideReal(double x) as Complex.divide(double x)
>> Complex.addReal(double x) as Complex.add(double x)
>> Complex.subtractReal(double x) as Complex.subtract(double x)
>>
>> Not covered:
>>
>> Complex.multiplyImaginary(double x)
>> Complex.divideImaginary(double x)
>> Complex.addImaginary(double x)
>> Complex.subtractImaginary(double x)
>> Complex.subtractFromReal(double x)
>> Complex.subtractFromImaginary(double x)
>>
>>
>> I am going through Complex now to fix code coverage and make it in line
>> with the C.99 standard. I will push all the config changes with the
>> update to Complex. Should be done be end of today.
>
>
>
> Well that's interesting, I did not see that the standard specified
> all-vs-all methods in all those cases. There isn't a performance gain for
> multiplying by an imaginary double like there is for a real double, as one
> has to deal with imaginary*imaginary, so one might as well just pass a
> Complex rather than an imaginary double. Consequently I would imagine
> implementations of that corner of the standard's matrix are pretty rare.
> But I see no reason not to have it for completeness and continuing the goal
> I set, of being the only non-C library that I've ever seen that fulfills
> the whole standard.
>
> +1
My changes to the tests for Complex have taken longer than I expected. I will continue tomorrow and will push something by the end of tomorrow.
One thing I noted is that although there is coverage of edge cases there is not a lot of coverage of the methods using ‘ordinary’ data. Some of the coverage comes from methods being used in other methods and not tested directly. I’ll fix up the code coverage and then will make an attempt to generate some results of complex operations using the GNU C compiler that can be incorporated into tests of the individual methods. This will be along the lines of:
void assertOperation(Complex z, UnaryOperator<Complex> op, Complex expected) {
// Check: expected == op.apply(z)
}
for all the operations on complex data.
I also note that the C.99 standard uses the following definitions of some functions using others:
casin(z) = -i casinh(iz)
catan(z) = -i catanh(iz)
ccos(z) = ccosh(iz)
csin(z) = -i csinh(iz)
ctan(z) = -i ctanh(iz)
The code currently was not doing this. It implemented the functions on the left directly. This means that all the edge cases in the function on the right (as these are tested in the CStandardTest) were missed from the function on the left. So for now I have implemented the equalities as defined above and commented out the original code.
Multiplication by I and -I is a straightforward real <=> imaginary swap with an appropriate sign update. So using the equalities is simple and more maintainable as some methods duplicate a lot of the same computation.
Alex
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Eric Barnhill <er...@gmail.com>.
On Thu, Nov 7, 2019 at 3:59 AM Alex Herbert <al...@gmail.com>
wrote:
>
> There is a matrix for real/imaginary/complex all-vs-all additive and
> multiplicative operators in the standard (tables in G.5.1 and G.5.2).
> The question is do we want to support the entire matrix:
>
> Covered:
>
> Complex.multiplyReal(double x) as Complex.multiply(double x)
> Complex.divideReal(double x) as Complex.divide(double x)
> Complex.addReal(double x) as Complex.add(double x)
> Complex.subtractReal(double x) as Complex.subtract(double x)
>
> Not covered:
>
> Complex.multiplyImaginary(double x)
> Complex.divideImaginary(double x)
> Complex.addImaginary(double x)
> Complex.subtractImaginary(double x)
> Complex.subtractFromReal(double x)
> Complex.subtractFromImaginary(double x)
>
>
> I am going through Complex now to fix code coverage and make it in line
> with the C.99 standard. I will push all the config changes with the
> update to Complex. Should be done be end of today.
Well that's interesting, I did not see that the standard specified
all-vs-all methods in all those cases. There isn't a performance gain for
multiplying by an imaginary double like there is for a real double, as one
has to deal with imaginary*imaginary, so one might as well just pass a
Complex rather than an imaginary double. Consequently I would imagine
implementations of that corner of the standard's matrix are pretty rare.
But I see no reason not to have it for completeness and continuing the goal
I set, of being the only non-C library that I've ever seen that fulfills
the whole standard.
+1
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
Le jeu. 7 nov. 2019 à 12:59, Alex Herbert <al...@gmail.com> a écrit :
>
>
> On 07/11/2019 11:45, Gilles Sadowski wrote:
> > Hello.
> >
> >>> [...]
> >>>
> >>>> Also looking at Complex it would benefit from:
> >>>>
> >>>> public Complex subtractFrom(double minuend) {
> >>>> return new Complex(minuend - real, imaginary);
> >>>> }
> >>> Is it part of the "standard"?
> >>> IMHO, it's fairly confusing, as nothing in the name indicates
> >>> that the operation only applies to the real part.
> >> The same applies for the add(real) and subtract(real) methods.
> >>
> >> What is confusing is the unconventional reverse ordering of z.subtractFrom(a) == a - z. I’m not happy about it but the javadoc can explain the outcome.
> > Actually, what confuses me is that the above equality does not hold:
> > double a = 5;
> > Complex z = Complex.of(1, 3);
> > Complex z2 = z.subtractFrom(a);
> > would yield
> > 4 + 3 i
> > whereas
> > Complex z2 = Complex.of(a).subtract(z);
> > would yield
> > 4 - 3 i
>
> You are right, I had my use of Complex incorrect. Sorry.
>
> The imaginary part should also be subtracted from zero thus the code
> should be an equivalent with an imaginary of 0:
>
> public Complex subtractFrom(double minuend) {
> return new Complex(minuend - real, 0 - imaginary);
> }
>
> Equivalent to (as you state):
>
> double a = ...;
>
> Complex z = ...;
>
> Complex y = Complex.ofReal(a).subtract(z);
So this would spare one instantiation.
Not sure it would gain much in absolute time for most uses, but there
is no arguing that it is a gain. [Although, if nit-picking, one could imagine
that the JIT optimization will kick in later if subtraction usage is spread
among the two methods, and thus could be a loss in absolute time for
some uses...]
> There is a matrix for real/imaginary/complex all-vs-all additive and
> multiplicative operators in the standard (tables in G.5.1 and G.5.2).
> The question is do we want to support the entire matrix:
This a more straightforward possible requirement.
> Covered:
>
> Complex.multiplyReal(double x) as Complex.multiply(double x)
> Complex.divideReal(double x) as Complex.divide(double x)
> Complex.addReal(double x) as Complex.add(double x)
> Complex.subtractReal(double x) as Complex.subtract(double x)
>
> Not covered:
>
> Complex.multiplyImaginary(double x)
> Complex.divideImaginary(double x)
> Complex.addImaginary(double x)
> Complex.subtractImaginary(double x)
> Complex.subtractFromReal(double x)
> Complex.subtractFromImaginary(double x)
I'm fine with adding these methods for consistency and completeness.
It may have the advantage to make it slightly easier to port algorithms
written in C++ if all the methods have their exact counterpart in Java.
Regards,
Gilles
>
>
> I am going through Complex now to fix code coverage and make it in line
> with the C.99 standard. I will push all the config changes with the
> update to Complex. Should be done be end of today.
>
> Then we can look at fixing checkstyle and PMD.
>
>
>>> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
On 07/11/2019 11:45, Gilles Sadowski wrote:
> Hello.
>
>>> [...]
>>>
>>>> Also looking at Complex it would benefit from:
>>>>
>>>> public Complex subtractFrom(double minuend) {
>>>> return new Complex(minuend - real, imaginary);
>>>> }
>>> Is it part of the "standard"?
>>> IMHO, it's fairly confusing, as nothing in the name indicates
>>> that the operation only applies to the real part.
>> The same applies for the add(real) and subtract(real) methods.
>>
>> What is confusing is the unconventional reverse ordering of z.subtractFrom(a) == a - z. I’m not happy about it but the javadoc can explain the outcome.
> Actually, what confuses me is that the above equality does not hold:
> double a = 5;
> Complex z = Complex.of(1, 3);
> Complex z2 = z.subtractFrom(a);
> would yield
> 4 + 3 i
> whereas
> Complex z2 = Complex.of(a).subtract(z);
> would yield
> 4 - 3 i
You are right, I had my use of Complex incorrect. Sorry.
The imaginary part should also be subtracted from zero thus the code
should be an equivalent with an imaginary of 0:
public Complex subtractFrom(double minuend) {
return new Complex(minuend - real, 0 - imaginary);
}
Equivalent to (as you state):
double a = ...;
Complex z = ...;
Complex y = Complex.ofReal(a).subtract(z);
There is a matrix for real/imaginary/complex all-vs-all additive and
multiplicative operators in the standard (tables in G.5.1 and G.5.2).
The question is do we want to support the entire matrix:
Covered:
Complex.multiplyReal(double x) as Complex.multiply(double x)
Complex.divideReal(double x) as Complex.divide(double x)
Complex.addReal(double x) as Complex.add(double x)
Complex.subtractReal(double x) as Complex.subtract(double x)
Not covered:
Complex.multiplyImaginary(double x)
Complex.divideImaginary(double x)
Complex.addImaginary(double x)
Complex.subtractImaginary(double x)
Complex.subtractFromReal(double x)
Complex.subtractFromImaginary(double x)
I am going through Complex now to fix code coverage and make it in line
with the C.99 standard. I will push all the config changes with the
update to Complex. Should be done be end of today.
Then we can look at fixing checkstyle and PMD.
>
> Regards,
> Gilles
>
>> It is possible in C:
>>
>> #include <stdio.h>
>> #include <complex.h>
>> #include <math.h>
>> #include <float.h>
>>
>> int main( int argc, const char* argv[] ) {
>> double a = 5.0;
>> complex double z = 1 + 3 * I;
>> complex double z2 = a - z;
>> printf("%.17g + %.17g i\n", creal(z2), cimag(z2));
>> complex double z3 = z - a;
>> printf("%.17g + %.17g i\n", creal(z3), cimag(z3));
>> }
>>
>> And C++:
>>
>> #include <complex>
>> #include <math.h>
>> #include <float.h>
>>
>> int main( int argc, const char* argv[] ) {
>> const double a = 5.0;
>> std::complex<double> z(1, 3);
>> std::complex<double> z2 = a - z;
>> printf("%.17g + %.17g i\n", z2.real(), z2.imag());
>> std::complex<double> z3 = z - a;
>> printf("%.17g + %.17g i\n", z3.real(), z3.imag());
>> }
>>
>> Both yield:
>>
>> 4 + -3 i
>> -4 + 3 i
>>
>> Note that Complex::add(double) exists. This needs no alternative as the addition of the real to the real component is commutative:
>>
>> double a
>> complex z
>> a + z = z + a
>>
>> The subtraction is not commutative (as shown above):
>> a - z != z - a
>>
>> So given the Complex has a subtract(double) it would be fair to have a subtractFrom(double).
>>
>> The complex subtraction is also not commutative:
>>
>> complex y
>> complex z
>> y - z != z - y
>>
>> But in this case you can achieve the same with the API by swapping the arguments.
>>
>>>> This would avoid:
>>>>
>>>> Complex a = …;
>>>> double b = …;
>>>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>>> This is clear.
>>>
>>>> Complex d = Complex.ofReal(b).subtract(a).conj();
>>> This even more, but, I get it, with 3 instantiations instead of 1.
>>>
>>>> With
>>>>
>>>> Complex a = …;
>>>> double b = …;
>>>> Complex c = a.subtractFrom(b);
>>> Looks ambiguous... But if it's standard naming.
>>>
>>> Regards,
>>> Gilles
>>>
>>>>>>
>>>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf>
>>>>>>
>>>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards <http://www.open-std.org/JTC1/SC22/WG14/www/standards>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
Hello.
> > [...]
> >
> >>
> >> Also looking at Complex it would benefit from:
> >>
> >> public Complex subtractFrom(double minuend) {
> >> return new Complex(minuend - real, imaginary);
> >> }
> >
> > Is it part of the "standard"?
> > IMHO, it's fairly confusing, as nothing in the name indicates
> > that the operation only applies to the real part.
>
> The same applies for the add(real) and subtract(real) methods.
>
> What is confusing is the unconventional reverse ordering of z.subtractFrom(a) == a - z. I’m not happy about it but the javadoc can explain the outcome.
Actually, what confuses me is that the above equality does not hold:
double a = 5;
Complex z = Complex.of(1, 3);
Complex z2 = z.subtractFrom(a);
would yield
4 + 3 i
whereas
Complex z2 = Complex.of(a).subtract(z);
would yield
4 - 3 i
Regards,
Gilles
> It is possible in C:
>
> #include <stdio.h>
> #include <complex.h>
> #include <math.h>
> #include <float.h>
>
> int main( int argc, const char* argv[] ) {
> double a = 5.0;
> complex double z = 1 + 3 * I;
> complex double z2 = a - z;
> printf("%.17g + %.17g i\n", creal(z2), cimag(z2));
> complex double z3 = z - a;
> printf("%.17g + %.17g i\n", creal(z3), cimag(z3));
> }
>
> And C++:
>
> #include <complex>
> #include <math.h>
> #include <float.h>
>
> int main( int argc, const char* argv[] ) {
> const double a = 5.0;
> std::complex<double> z(1, 3);
> std::complex<double> z2 = a - z;
> printf("%.17g + %.17g i\n", z2.real(), z2.imag());
> std::complex<double> z3 = z - a;
> printf("%.17g + %.17g i\n", z3.real(), z3.imag());
> }
>
> Both yield:
>
> 4 + -3 i
> -4 + 3 i
>
> Note that Complex::add(double) exists. This needs no alternative as the addition of the real to the real component is commutative:
>
> double a
> complex z
> a + z = z + a
>
> The subtraction is not commutative (as shown above):
> a - z != z - a
>
> So given the Complex has a subtract(double) it would be fair to have a subtractFrom(double).
>
> The complex subtraction is also not commutative:
>
> complex y
> complex z
> y - z != z - y
>
> But in this case you can achieve the same with the API by swapping the arguments.
>
> >
> >>
> >> This would avoid:
> >>
> >> Complex a = …;
> >> double b = …;
> >> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
> >
> > This is clear.
> >
> >> Complex d = Complex.ofReal(b).subtract(a).conj();
> >
> > This even more, but, I get it, with 3 instantiations instead of 1.
> >
> >>
> >> With
> >>
> >> Complex a = …;
> >> double b = …;
> >> Complex c = a.subtractFrom(b);
> >
> > Looks ambiguous... But if it's standard naming.
> >
> > Regards,
> > Gilles
> >
> >>
> >>>
> >>>>
> >>>>
> >>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf>
> >>>>
> >>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards <http://www.open-std.org/JTC1/SC22/WG14/www/standards>
> >>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 7 Nov 2019, at 00:28, Gilles Sadowski <gi...@gmail.com> wrote:
>
> 2019-11-06 23:38 UTC+01:00, Alex Herbert <alex.d.herbert@gmail.com <ma...@gmail.com>>:
>>
>>
>>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>>>
>>>> [...]
>>>>
>>>>
>>>> Any objections to updating multiply/divide/isNaN to match the standard?
>>>
>>> Let me think... ;-)
>>
>> OK, I’ll fix it and double check the other tests against the c reference.
>>
>>>
>>>>
>>>> I'll add unit tests to hit the edge cases that should fail with the
>>>> current implementation.
>>>
>>> Thanks,
>>> Gilles
>>
>> Are changes to numbers going under Jira tickets?
>
> There is a JIRA project:
> https://issues.apache.org/jira/projects/NUMBERS <https://issues.apache.org/jira/projects/NUMBERS>
> for reference in the git commit (for non-trivial changes).
>
> But we don't really keep track (no "changes.xml") until the first
> official release (getting close now, I hope...).
>
>>
>> It looks like it needs an update to checkstyle, PMD, spotbugs, the
>> commons-parent and travis.
>
> Yes. I try to copy from or refer to Commons RNG.
>
>> Checkstyle config from commons-rng finds:
>>
>> [INFO] There are 115 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 202 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 102 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 276 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 68 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 289 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 56 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 50 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>>
>> The mass of errors is white space style in the test classes.
>
> The bulk of the tests was ported from Commons Math where
> CheckStyle was not enforced for unit test classes.
>
>> Without the
>> test classes the result is:
>>
>> [INFO] There are 12 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 54 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 49 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 5 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 20 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>
> Yes, those should be fixed. It's on the TODO list (among other tasks):
> https://issues.apache.org/jira/projects/NUMBERS/issues/NUMBERS-25 <https://issues.apache.org/jira/projects/NUMBERS/issues/NUMBERS-25>
>
>>
>> Also looking at Complex it would benefit from:
>>
>> public Complex subtractFrom(double minuend) {
>> return new Complex(minuend - real, imaginary);
>> }
>
> Is it part of the "standard"?
> IMHO, it's fairly confusing, as nothing in the name indicates
> that the operation only applies to the real part.
The same applies for the add(real) and subtract(real) methods.
What is confusing is the unconventional reverse ordering of z.subtractFrom(a) == a - z. I’m not happy about it but the javadoc can explain the outcome.
It is possible in C:
#include <stdio.h>
#include <complex.h>
#include <math.h>
#include <float.h>
int main( int argc, const char* argv[] ) {
double a = 5.0;
complex double z = 1 + 3 * I;
complex double z2 = a - z;
printf("%.17g + %.17g i\n", creal(z2), cimag(z2));
complex double z3 = z - a;
printf("%.17g + %.17g i\n", creal(z3), cimag(z3));
}
And C++:
#include <complex>
#include <math.h>
#include <float.h>
int main( int argc, const char* argv[] ) {
const double a = 5.0;
std::complex<double> z(1, 3);
std::complex<double> z2 = a - z;
printf("%.17g + %.17g i\n", z2.real(), z2.imag());
std::complex<double> z3 = z - a;
printf("%.17g + %.17g i\n", z3.real(), z3.imag());
}
Both yield:
4 + -3 i
-4 + 3 i
Note that Complex::add(double) exists. This needs no alternative as the addition of the real to the real component is commutative:
double a
complex z
a + z = z + a
The subtraction is not commutative (as shown above):
a - z != z - a
So given the Complex has a subtract(double) it would be fair to have a subtractFrom(double).
The complex subtraction is also not commutative:
complex y
complex z
y - z != z - y
But in this case you can achieve the same with the API by swapping the arguments.
>
>>
>> This would avoid:
>>
>> Complex a = …;
>> double b = …;
>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>
> This is clear.
>
>> Complex d = Complex.ofReal(b).subtract(a).conj();
>
> This even more, but, I get it, with 3 instantiations instead of 1.
>
>>
>> With
>>
>> Complex a = …;
>> double b = …;
>> Complex c = a.subtractFrom(b);
>
> Looks ambiguous... But if it's standard naming.
>
> Regards,
> Gilles
>
>>
>>>
>>>>
>>>>
>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf>
>>>>
>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards <http://www.open-std.org/JTC1/SC22/WG14/www/standards>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org <ma...@commons.apache.org>
> For additional commands, e-mail: dev-help@commons.apache.org <ma...@commons.apache.org>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
2019-11-06 23:38 UTC+01:00, Alex Herbert <al...@gmail.com>:
>
>
>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>>
>>> [...]
>>>
>>>
>>> Any objections to updating multiply/divide/isNaN to match the standard?
>>
>> Let me think... ;-)
>
> OK, I’ll fix it and double check the other tests against the c reference.
>
>>
>>>
>>> I'll add unit tests to hit the edge cases that should fail with the
>>> current implementation.
>>
>> Thanks,
>> Gilles
>
> Are changes to numbers going under Jira tickets?
There is a JIRA project:
https://issues.apache.org/jira/projects/NUMBERS
for reference in the git commit (for non-trivial changes).
But we don't really keep track (no "changes.xml") until the first
official release (getting close now, I hope...).
>
> It looks like it needs an update to checkstyle, PMD, spotbugs, the
> commons-parent and travis.
Yes. I try to copy from or refer to Commons RNG.
> Checkstyle config from commons-rng finds:
>
> [INFO] There are 115 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 202 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 102 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 276 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 68 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 289 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 10 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 56 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 50 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 10 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 4 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
>
> The mass of errors is white space style in the test classes.
The bulk of the tests was ported from Commons Math where
CheckStyle was not enforced for unit test classes.
> Without the
> test classes the result is:
>
> [INFO] There are 12 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 54 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 19 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 49 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 5 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 6 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 3 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 20 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 19 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 4 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
> [INFO] There are 6 errors reported by Checkstyle 8.20 with
> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
> ruleset.
Yes, those should be fixed. It's on the TODO list (among other tasks):
https://issues.apache.org/jira/projects/NUMBERS/issues/NUMBERS-25
>
> Also looking at Complex it would benefit from:
>
> public Complex subtractFrom(double minuend) {
> return new Complex(minuend - real, imaginary);
> }
Is it part of the "standard"?
IMHO, it's fairly confusing, as nothing in the name indicates
that the operation only applies to the real part.
>
> This would avoid:
>
> Complex a = …;
> double b = …;
> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
This is clear.
> Complex d = Complex.ofReal(b).subtract(a).conj();
This even more, but, I get it, with 3 instantiations instead of 1.
>
> With
>
> Complex a = …;
> double b = …;
> Complex c = a.subtractFrom(b);
Looks ambiguous... But if it's standard naming.
Regards,
Gilles
>
>>
>>>
>>>
>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>>
>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Alex Herbert <al...@gmail.com>.
> On 6 Nov 2019, at 18:17, Gilles Sadowski <gi...@gmail.com> wrote:
>
>> [...]
>>
>>
>> Any objections to updating multiply/divide/isNaN to match the standard?
>
> Let me think... ;-)
OK, I’ll fix it and double check the other tests against the c reference.
>
>>
>> I'll add unit tests to hit the edge cases that should fail with the
>> current implementation.
>
> Thanks,
> Gilles
Are changes to numbers going under Jira tickets?
It looks like it needs an update to checkstyle, PMD, spotbugs, the commons-parent and travis.
Checkstyle config from commons-rng finds:
[INFO] There are 115 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 202 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 102 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 276 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 68 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 289 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 10 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 3503 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 56 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 50 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 10 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 4 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml ruleset.
The mass of errors is white space style in the test classes. Without the test classes the result is:
[INFO] There are 12 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 54 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 19 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 49 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 5 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 6 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 3 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 20 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 19 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 4 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml ruleset.
[INFO] There are 6 errors reported by Checkstyle 8.20 with /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml ruleset.
Also looking at Complex it would benefit from:
public Complex subtractFrom(double minuend) {
return new Complex(minuend - real, imaginary);
}
This would avoid:
Complex a = …;
double b = …;
Complex c = Complex.ofCartesian(b - a.real(), a.imag());
Complex d = Complex.ofReal(b).subtract(a).conj();
With
Complex a = …;
double b = …;
Complex c = a.subtractFrom(b);
>
>>
>>
>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>
>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
Re: [numbers] Bug in complex multiply + divide + isNaN
Posted by Gilles Sadowski <gi...@gmail.com>.
> [...]
>
>
> Any objections to updating multiply/divide/isNaN to match the standard?
Let me think... ;-)
>
> I'll add unit tests to hit the edge cases that should fail with the
> current implementation.
Thanks,
Gilles
>
>
> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>
> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org