Description: a 'deflowering'? based on 'search' no bugs have been reported for 'roundup' and 'roundown' yet? i stumbled over one: =ROUNDUP(-999,12999999999994;11) results in -999,130000000010 instead of the expected -999,130000000000, =ROUNDDOWN(-999,12999999999994;11) results in -999,130000000000 instead of the expected -999.12999999999990 same fail - mirrored - for positive values, for those not familiar with it: roundup should round 'away from zero', rounddown 'towards zero', plenty other values affected as well, even shorter ones like '888,1299999999' which need no change for a round to 11 decimals ... 'round' looks working correctly on a quick first look, no repro in 6.1.6.3, hence 'regression', imho critical, data corruption ... Steps to Reproduce: see above description Actual Results: see above description Expected Results: see above description Reproducible: Always User Profile Reset: No Additional Info: Version: 7.1.0.0.alpha1+ (x64) Build ID: b61bf7c7cfcf97a5ade6d130873af146670bc2ee CPU threads: 8; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: default; VCL: win Locale: de-DE (de_DE); UI: en-US Calc: CL 'unparallelized' (without openCL and threading) same effect,
above reported fail no repro in 6.1.6.3, but =ROUNDUP(8000000000000000;-15) results in 7.999.999.999.999.999 there, thus it's not been 'clean' in the past ... no 'fp-imprecision' neccessary in that range, and ... rounding should in some way help to cover imprecision, not introduce additional fails ...
This seems to have begun at the below commit. Adding Cc: to Eike Rathke ; Could you possibly take a look at this one? Thanks 4ac0c8c10e88ac1ea470606e0d6973f8171c8516 is the first bad commit commit 4ac0c8c10e88ac1ea470606e0d6973f8171c8516 Author: Jenkins Build User <tdf@pollux.tdf> Date: Thu Mar 28 23:23:07 2019 +0100 source edcbe8c4e02a67c74ec6f85f28899431dbfa0765 https://gerrit.libreoffice.org/c/core/+/69762 tdf#124286 fix annoying rounding error. In case of ROUNDDOWN and ROUNDUP, it is possible that seemingly clear decimal values are rounded unexpectedly (from the user's POV). This is caused by the i decimal to binary to decimal conversions. By rounding to 12 significanr digits before calling the round-down of -up function, most of these unexpected roundings are eliminated. Change-Id: Ia19181383b77e1ff40a067c4a1cea1ece0955871 Reviewed-on: https://gerrit.libreoffice.org/69762 Tested-by: Jenkins Reviewed-by: Eike Rathke <erack@redhat.com>
Input of =ROUNDUP(-999.12999999999994;11) results in -999.13 and the formula expression becomes =ROUNDUP(-999.13;11) If you force a 0.000000000000 number format the resulting -999.130000000010 is a display string conversion problem, not a wrong calculation. Input of =ROUNDDOWN(-999.12999999999994;11) results in -999.13 and the formula expression becomes =ROUNDDOWN(-999.13;11) The input of -999.12999999999994 can't be represented in IEEE-754 double floating-point and yields -999.13 The nearest representable value of -999.12999999999994 is -999.1299999999999954525264911353588104248046875 with a raw hex value of c08f390a3d70a3d7 (binary exponent.mantissa -1111100111.0010000101000111101011100001010001111010111) which is the same nearest value as for -999.13 Do not expect rounding down to whatever decimals would yield any -999.12999999999990 This is no way "critical, data corruption", it is how IEEE-754 double floating-point works. I also get no different results in 6.1.6 apart from that the forced 12 decimals formatted string for the first case is -999.130000000000 @zcrhonek: what and how did you test there to get to the bisected result?
Created attachment 167728 [details] test file Eike, test file.
the display strings should / could be better? '=ROUNDUP(999,1300000004;2)' -> 999,13000000000 is not ok?, '=ROUNDDOWN(999,1299999995;2)' -> 999,13000000000 is not ok?, above results from 7.2.0.0.a0+, both better with 6.1.6.3, sorry, OP was just the first occurence without more research,
@erAck: yes, but '=ROUNDUP(-999,13;11)' shouldn't end in -999,13000000001000000000 either... right? but it does :-( and it's not just 'display string', see: =RAWSUBTRACT(ROUNDUP(-999,13;11);-999,13) -> -0,00000000001000444172 vs.: '=RAWSUBTRACT(-999,13;-999,13)' -> 0,0000000000000000 and: '=RAWSUBTRACT(ROUND(-999,13;2);-999,13)' -> 0,00000000000000000000 there are two distinct issues: 1. the patch by @Winfried Donkers - https://gerrit.libreoffice.org/c/core/+/69762 - disables accuracy for values with more than 12 significant digits, such is a very questionable strategy, i would suggest to undo the patch and fix the errors in the elementary functions instead (roundsig and subtraction)), 2. something injects a small deviation into these calculations, imho it starts with roundsig being inaccurate, see: '=RAWSUBTRACT(ROUNDSIG(-999,13;12);-999,13)' -> -0,00000000000011368684 ('RoundSignificant( fX, 12, fRes );' is used in the patch)
it sounds like magic, but with that little change: // tdf124286 : round to 12 significant digits before rounding // down or up to avoid unexpected rounding errors // caused by decimal -> binary -> decimal conversion // [edit b. 2021-05-22: try with roundsig 15 instead of roundsig 12 to keep precision] double fRes; //ori was: RoundSignificant( fX, 12, fRes ); RoundSignificant( fX, 15, fRes ); fVal = ::rtl::math::round( fRes, nDec, eMode ); instead of: // tdf124286 : round to 12 significant digits before rounding // down or up to avoid unexpected rounding errors // caused by decimal -> binary -> decimal conversion double fRes; RoundSignificant( fX, 12, fRes ); fVal = ::rtl::math::round( fRes, nDec, eMode ); in sc/source/core/tool/interpr2.cxx the above issues seem gone, while the 'healing' against the cancellation issue from tdf#124286 (8,94-8) still holds ... would one of the more experienced users or devs like to: - recheck, - if good lead me through the process of implementing it as a patch? (tried with Kali (Debian) Linux and master source from 2021-05-20)
It breaks the examples you gave in https://bugs.documentfoundation.org/show_bug.cgi?id=124286#c17 =ROUNDDOWN(2-5E-015;14) -> 1.99999999999999 =ROUNDDOWN(2-5E-015;13) -> 1.9999999999999 Both are then 2, also if nDec < 12 is changed to nDec < 15. However, process to setup Gerrit and submit patches is described at https://wiki.documentfoundation.org/Development/gerrit
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/397d12997a604730ad11316faa34cefd470ee0ff ROUNDSIG() Avoid inaccuracy of pow(10,negative) tdf#138220, tdf#105931 follow It will be available in 7.6.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.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/8186a01f2a26f05645a2a3c9c93b453bd35b796f Resolves: tdf#138220 tdf#154792 Avoid double rounding; tdf#124286 follow-up It will be available in 7.6.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.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/e68cd91651c53c5c228d84582b1062d8fb9a7077 Add ROUND(DOWN|UP) samples to unit test, tdf#154792 tdf#124286 tdf#138220 It will be available in 7.6.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.
Eike Rathke committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/a062bceb2509d227cec86845af680a4b69d42df3 Add ROUNDSIG samples to unit test, tdf#138220 tdf#105931 It will be available in 7.6.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.
Eike Rathke committed a patch related to this issue. It has been pushed to "libreoffice-7-5": https://git.libreoffice.org/core/commit/f0f2ceaf978a0eee8e3c1fcfe6dec546ab8f07d6 Resolves: tdf#138220 tdf#154792 Avoid double rounding; tdf#124286 follow-up It will be available in 7.5.4. 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.
Eike Rathke committed a patch related to this issue. It has been pushed to "libreoffice-7-5": https://git.libreoffice.org/core/commit/81458d155a442a98dac0e30ce36725c15901f7a9 ROUNDSIG() Avoid inaccuracy of pow(10,negative) tdf#138220, tdf#105931 follow It will be available in 7.5.4. 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.