| Summary: | 'mod(13,12)' method in MathX should produce '1' | ||
|---|---|---|---|
| Product: | POI | Reporter: | java |
| Component: | XSSF | Assignee: | POI Developers List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | ||
| Priority: | P2 | ||
| Version: | 3.7-dev | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Attachments: |
Patched MathX based on version 776505 (3.7b3)
Test case which exposes the issue. |
||
Created attachment 26107 [details]
Test case which exposes the issue.
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 |
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.