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 commonsnumber 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 apacherat:check clirr:check checkstyle:check findbugs:check
> javadoc:javadoc" which is not happening right now with
> commonsnumber.
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/NUMBERS43
So this issue might automagically go away. ;)
> I know commonsnumber 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/NUMBERS25
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, email: devunsubscribe@commons.apache.org
For additional commands, email: devhelp@commons.apache.org
