Bug 158190 - Calc ROUND() function issue in specific range
Summary: Calc ROUND() function issue in specific range
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:24.8.0
Keywords:
Depends on:
Blocks: Cell-Formula
  Show dependency treegraph
 
Reported: 2023-11-13 04:03 UTC by Po-Yen Huang
Modified: 2024-03-21 17:02 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
As bug example (10.72 KB, application/vnd.oasis.opendocument.spreadsheet)
2023-11-13 04:04 UTC, Po-Yen Huang
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Po-Yen Huang 2023-11-13 04:03:24 UTC
Description:
For math formula in ROUND() function:
1. If the integer is below 15 (inclusive) or above 25 (inclusive),
the result will be correct regardless of the decimal.

2. The integer part falls between 16 and 24. As long as the first
decimal is 3 or 8, the ROUND() function will not round.

But if ROUND a number but not formula, we can get correct result.

Steps to Reproduce:
1. Insert a number in a cell like description above (for example, 16.83 or 16.33)
2. Insert a number (that can be multiplied with step 1 to X.5) in another cell
3. Insert =ROUND(A1*B1,0) in another cell (multiply step 1 and step 2 number and ROUND it)

Actual Results:
It seems to be rounding down (for example, ROUND(16.33*650) shows 10614)

Expected Results:
It should be rounding (for example, ROUND(16.33*650) should be 10615)


Reproducible: Always


User Profile Reset: Yes

Additional Info:
Version: 7.6.2.1 (X86_64) / LibreOffice Community
Build ID: 60(Build:1)
CPU threads: 8; OS: Linux 6.6; UI render: default; VCL: kf5 (cairo+xcb)
Locale: zh-TW (zh_TW.UTF-8); UI: zh-TW
7.6.2-3
Calc: threaded
Comment 1 Po-Yen Huang 2023-11-13 04:04:58 UTC
Created attachment 190801 [details]
As bug example
Comment 2 Mike Kaganski 2023-11-13 04:44:24 UTC
As you correctly identified in https://gerrit.libreoffice.org/c/core/+/159193, it is caused by a special handling of nDecPlaces == 0 with rtl_math_RoundingMode_Corrected in rtl_math_round, when it doesn't use rtl::math::approxFloor, as used with other values of nDecPlaces. This makes the nDecPlaces == 0 ti actually not use "corrected" behavior, as defined in the API [1]: 

> Like HalfUp, but corrects roundoff errors

The special casing of nDecPlaces == 0 with rtl_math_RoundingMode_Corrected looks unneeded there at all, so as I suggested there in the change, the preferred solution would be not fixing the special casing, but removing it, and allowing the general case code to do its job.

Thank you!

[1] https://opengrok.libreoffice.org/xref/core/include/rtl/math.h?r=b1fb6338&mo=2922&fi=108#106
Comment 3 Mike Kaganski 2023-11-13 06:31:44 UTC
Note that the range you identified is not something special; e.g., the same error would happen with

=ROUND(1.143541958418614 * 9283; 0)

and the only thing that matters is that the floating-point number is very close to X.5, but a bit less, by a few ULPs - so something like 10939.499999999998. The range between 16 and 24 makes the specific multiplier of 650 expose many of these; but for a different multiplier, a different range would exhibit the same problem.
Comment 4 Mike Kaganski 2023-11-13 07:30:20 UTC
Note that the special casing was introduced for bug 96821 in commit b97a0df0f3234b4c1140ba1418d4b96a592afa4a, to mitigate rtl::math::round issues for large numbers like 2^52+1; and at that time, the general case code doing rounding wasn't guarded by conditions introduced later in commit ecfcd99abd3f7dfe68a306dd8045d2da79e42d74. The latter commit mage the former one obsolete.
Comment 5 mwelinder 2023-11-13 17:33:43 UTC
> Like HalfUp, but corrects roundoff errors

That's a game of whack-a-mole.  It is not a solvable problem as long as the math is being done in base 2.

=(16.33*100650)-1633000
--> 10614.4999999998

What do you want that one to round to?

=(16.33*1000650)-16330000
--> 10614.4999999981

That one?  You can make the rounding error as big as you like.
Comment 6 Mike Kaganski 2023-11-13 17:40:31 UTC
(In reply to mwelinder from comment #5)
> That's a game of whack-a-mole.  It is not a solvable problem as long as the
> math is being done in base 2.

Sure. But what is your point here? There is at least the expectation that it works like in other major spreadsheet programs. Excel does an approximation. Gnumeric does an approximation. We do an approximation. It should be ~reasonable, and it is to some extent. Bugs get fixed.
Comment 7 Mike Kaganski 2023-11-24 11:20:05 UTC Comment hidden (obsolete)
Comment 8 Commit Notification 2024-02-27 10:01:28 UTC
Po-Yen Huang committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9adffb1c7453aee77f9d52b1ed0aa99191a9c317

tdf#158190 Fix Calc ROUND in floating-point calculate result very close to X.5

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.