Hi Amey.
On Sun, 11 Jun 2017 23:30:04 +0530, Amey wrote:
Hi Gilles,
On Thursday 08 June 2017 03:22 AM, Gilles wrote:
Hello.
On Wed, 7 Jun 2017 23:52:59 +0530, Amey Jadiye wrote:
Hi,
I was trying to run all checks on commons-number and found findbug
is
failing in Precision.java[544] FE_FLOATING_POINT_EQUALITY
{code}
case BigDecimal.ROUND_HALF_EVEN : {
double fraction = unscaled - Math.floor(unscaled);
if (fraction > 0.5) {
unscaled = Math.ceil(unscaled);
} else if (fraction < 0.5) {
unscaled = Math.floor(unscaled);
} else {
// The following equality test is intentional and
needed
for rounding purposes
if (Math.floor(unscaled) / 2.0 ==
Math.floor(Math.floor(unscaled) / 2.0)) { // even // failing here.
//
unscaled = Math.floor(unscaled);
} else { // odd
unscaled = Math.ceil(unscaled);
}
}
break;
}
{code}
Error is :
Test for floating point equality in
org.apache.commons.numbers.core.Precision.roundUnscaled(double,
double,
int) [Of Concern(15), High confidence]
Fix:
Replace equality check with below:
if (Math.abs((Math.floor(unscaled) / 2.0) -
(Math.floor(Math.floor(unscaled) / 2.0))) < .0000001)
Why ".0000001"?
My intention was to use EPSILON, which should be the arbitrarily
small
positive quantity, anyway I dropped the way I proposed which is
explained below.
we have couple of similar issues in code.
Let me know if we have better alternative, else will submit code.
Why do you think that the strict equality check must be replaced
since there is a comment indicating that it is "intentional"?
[I mean, is there an identified problem with the code as it is
now, apart from the "FindBugs" assertion?]
This is because doubles and floats cannot express every numerical
value.
They are really using approximations to represent the value, so
recommended way is to compare difference of both values with some low
value(epsilon) OR better we use Double.compare(d1, d2) ?
Note that sometimes, a strict equality check can be part of
the algorithm logic. [I don't know whether it's the case here.]
There is no problem with code but I strongly wish we *must* pass "mvn
test apache-rat:check clirr:check checkstyle:check findbugs:check
javadoc:javadoc" which is not happening right now with
commons-number.
Certainly; but _if_ it's a case as mentioned above, this is
achieved by "telling" the tool that the code is known to be
correct.
Anyway, I've opened
https://issues.apache.org/jira/browse/NUMBERS-43
So this issue might automagically go away. ;-)
I know commons-number is not released yet but there are some other
minor
findbugs issue we should fix.
Please mention them all here:
https://issues.apache.org/jira/browse/NUMBERS-25
and/or file JIRA reports.
Thank you,
Gilles
ex. In ComplexUtils, for couple of methods(complex2Interleaved,
interleaved2Complex) we are creating new exception but not throwing
them
? seems valid bug to me.
Thanks,
Gilles
Regards,
Amey
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
