You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2022/04/29 12:10:43 UTC

[Bug 66047] New: Rounding issue in MROUND due to 1897066

https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

            Bug ID: 66047
           Summary: Rounding issue in MROUND due to 1897066
           Product: POI
           Version: 5.2.1-FINAL
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: SS Common
          Assignee: dev@poi.apache.org
          Reporter: fabio.heer@dvbern.ch
  Target Milestone: ---

Created attachment 38271
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38271&action=edit
MRound BigDecimal implementation

Hi

I noticed a rounding difference after upgrading to poi and tracked the change
back to version 5.2.1
https://svn.apache.org/viewvc?view=revision&revision=1897066.

I do acknowledge the new calculation as more correct, but it creates a follow
up problem when using the MROUND formula.

Consider the following example: MROUND(0.79*7.5;0.05).
In Excel, the result is 5.95. In POI 5.2.1 the result ist 5.90.

This happens because earlier POI version evaluated 0.79*7.5 to
5.925000000000001. The new BigDecimal based calculation ist instead (correctly)
evaluating this to 5.925.

The MRound implementation is rounding this down, where as before it was
rounding up:
> result = multiple * Math.round( number / multiple );

In my opinion, it would make sense to replace this implementation with a
BigDecimal version and the HALF_UP rounding mode (see patch).

Thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

--- Comment #3 from Fabio <fa...@dvbern.ch> ---
Hi

I have second thoughts about the rounding improvements in
https://svn.apache.org/viewvc?view=revision&revision=1897066.

My patch does solve some cases, but it also does something different than the
excel implementation. With my patch, the example of MS' "Known Limitations"
breaks. Both, 6.05 and 7.05 are now rounded deterministically to 6.1
respectively 7.1. The example from Microsoft rounds to 6.0 and 7.1.

In my opinion, consistent rounding is definitely better, but then the POI
implementation is not consistent with Excel :-(
Finding quirks to accommodate for Excel's undefined behaviour is error prone
and unmaintainable.

I don't know what's the best way forward here. Reverting 1897066 is probably
not an option either, is it?

[1]
https://support.microsoft.com/en-us/office/mround-function-c299c3b0-15a5-426d-aa4b-d2d5b3baf427

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

--- Comment #1 from Fabio <fa...@dvbern.ch> ---
Created attachment 38272
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=38272&action=edit
Excel File with rounding issue

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

--- Comment #4 from Axel Howind <ax...@dua3.com> ---
I am not sure if this is really more consistent. As I have said in a comment to
another issue, there is no exact 6.05 and no exact 7.05 in excel as excel
internally uses IEEE754 numbers. Both numbers are represented by their nearest
repesentable neighbour, and the fractional part of both representations ($1 and
$2) is < 0.05, so a consistent solution would round both numbers down. What
happens here is that the errors in the floating point representations of 7.05
($2) and 0.1 ($3) cancel each other out and lead to the result 7.1 ($5 rounded
to one decimal).

You can check this in jshell by using the double constructor of BigDecimal
(note that you should avoid that constructor at all costs for other purposes):


```
    % jshell 
    |  Welcome to JShell -- Version 18.0.1
    |  For an introduction type: /help intro

    jshell> new BigDecimal(6.05)
    $1 ==> 6.04999999999999982236431605997495353221893310546875

    jshell> new BigDecimal(7.05)
    $2 ==> 7.04999999999999982236431605997495353221893310546875

    jshell> new BigDecimal(0.1)
    $3 ==> 0.1000000000000000055511151231257827021181583404541015625

    jshell> new BigDecimal(6.05*0.1)
    $4 ==> 0.604999999999999982236431605997495353221893310546875

    jshell> new BigDecimal(7.05*0.1)
    $5 ==> 0.7050000000000000710542735760100185871124267578125
```

This is the reason why you get 6.0 and 7.1 respectively in Excel.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

--- Comment #5 from PJ Fanning <fa...@yahoo.com> ---
I'm currently travelling so can't devote much time at the moment for this. I
would prefer not to revert the previous change. In practice, getting POI code
in Java to act like Excel does is 100% of cases is a real pain and we should
drive any solutions by using test cases based on observed behaviour in Excel.

Your best solution is to set wb.setForceFormulaRecalculation(true); - so that
Excel will recalc all the values when it loads the workbook.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

PJ Fanning <fa...@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #2 from PJ Fanning <fa...@yahoo.com> ---
thanks - patch applied with r1900376

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[Bug 66047] Rounding issue in MROUND due to 1897066

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=66047

--- Comment #6 from Yegor Kozlov <ye...@gmail.com> ---
I'd say POI is good and on par with other implementations. Google Sheets and
LibreOffice consistently evaluate MROUND(6.05,0.1) to 6.1 and MROUND(7.05,0.1)
to 7.1.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org