commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [NUMBERS] : findbug failing in commons-number-core
Date Mon, 12 Jun 2017 12:44:58 GMT
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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message