Bug 50033 - 'mod(13,12)' method in MathX should produce '1'
Summary: 'mod(13,12)' method in MathX should produce '1'
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.7-dev
Hardware: PC Windows XP
: P2 major (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-30 14:03 UTC by java
Modified: 2010-10-01 08:04 UTC (History)
0 users



Attachments
Patched MathX based on version 776505 (3.7b3) (12.68 KB, patch)
2010-09-30 14:03 UTC, java
Details | Diff
Test case which exposes the issue. (29.07 KB, text/x-java)
2010-09-30 14:04 UTC, java
Details

Note You need to log in before you can comment on or make changes to this bug.
Description java 2010-09-30 14:03:56 UTC
Created attachment 26106 [details]
Patched MathX based on version 776505 (3.7b3)

The formula 'mod(13,12)' entered into Excel produces the answer of '1'.  POI is evaluating 'mod(13,12)' to '0.9999999999999991'.

I have attached a patched version of MathX based on version 776505 which fixes the problem.  Also attached is an updated TestMathX with the previously empty 'testMod' method filled in.  It still has some oddities when one or the other values are negative (returning '0.6000000000000001' vs '0.6' for 'mod(-3.4,2.0)' for instance), but that was a pre-existing condition prior to my change.
Comment 1 java 2010-09-30 14:04:42 UTC
Created attachment 26107 [details]
Test case which exposes the issue.
Comment 2 Yegor Kozlov 2010-10-01 08:04:02 UTC
Thanks for the patch, applied in r1003504

The fact that in some cases the results are slightly 'off' is normal, it is the way floating-point arithmetic works. In particular, the value of 1.4 cannot be represented as an exact value, on low-level it is a result of a truncated series. A way to check it is via BigDecimal(1.4).toString() which returns 3.399999999999999911182158029987476766109466552734375. 


A smarter version of MathX.mod might use BigDecimal for calculations:

    public static double mod(double n, double d) {
        BigDecimal number = new BigDecimal(n);
        BigDecimal divisor = new BigDecimal(d);

        double result;

        if (d == 0) {
            result = Double.NaN;
        }
        else if (sign(n) == sign(d)) {
            result = number.remainder(divisor).doubleValue();
        }
        else {
            result = number.remainder(divisor).add(divisor).remainder(divisor).doubleValue();
        }

        return result;
    }



In the case of MathX.mode this seems OK, but I'm reluctant to make this change. Using BigDecimal for calculations should be project-wide.

Regards,
Yegor