Bug 125580 - Slightly off value when adding date plus time values
Summary: Slightly off value when adding date plus time values
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.2.4.2 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:24.2.0 target:7.6.4
Keywords: bibisected, bisected
: 127143 (view as bug list)
Depends on:
Blocks: Formula
  Show dependency treegraph
 
Reported: 2019-05-29 14:00 UTC by Alexandre Rio
Modified: 2023-11-22 20:58 UTC (History)
10 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot of the bug happening (49.44 KB, image/png)
2019-05-29 14:01 UTC, Alexandre Rio
Details
time_rounding_difference (343.37 KB, image/png)
2019-05-30 06:30 UTC, Oliver Brinzing
Details
time_rounding (23.52 KB, application/vnd.oasis.opendocument.spreadsheet-template)
2019-05-30 06:30 UTC, Oliver Brinzing
Details
time_rounding.xlsx (6.11 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2019-05-30 06:31 UTC, Oliver Brinzing
Details
sheet to show accurate fill series are possible; (3.82 MB, application/vnd.oasis.opendocument.spreadsheet)
2021-04-05 01:14 UTC, b.
Details
testsheet for 'fills' with fractional values (4.27 MB, application/vnd.oasis.opendocument.spreadsheet)
2021-04-13 08:28 UTC, b.
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Rio 2019-05-29 14:00:53 UTC
Description:
When using the + operator on dates multiple times the result ends up being erroneous.

Steps to Reproduce:
1. Enter =FLOOR(NOW(), "00:05") in cell A1 to have "clean" date
2. Enter =A1 + TIME(0;5;0)
3. Use the handle to apply the function multiple times

Actual Results:
After 20 lines the output becomes:

29/05/19 17:05
29/05/19 17:10
29/05/19 17:14
29/05/19 17:19

Meaning 4 minutes have been added instead of 5

Expected Results:
Always have the same value added on every row


Reproducible: Always


User Profile Reset: No



Additional Info:
Bug is also present when using a plain text date on first cell and formatting as a date but doesn't work with only =NOW() function in first cell.
Same behavior when using "+ (1/24/12)" as increment.

Version: 6.2.4.2.0+
Build ID: 6.2.4-1
CPU threads: 8; OS: Linux 5.1; UI render: default; VCL: gtk3; 
Locale: fr-FR (en_US.UTF-8); UI-Language: en-US
Calc: threaded
Comment 1 Alexandre Rio 2019-05-29 14:01:36 UTC
Created attachment 151762 [details]
Screenshot of the bug happening
Comment 2 m_a_riosv 2019-05-29 21:22:13 UTC
This is a rounding issue.
date-time value have a real value in the cells:
days as integer number + hours/24 + minutes/24/60 + seconds/24/60/60
The visualization format doesn't change the cell value.

The first thing is using NOW() it gives actual date+time

So better TODAY() that gives an integer value
=ROUNDUP(TODAY()+TIME(0;5;0);7) in B1
 a formula like
=ROUNDUP(B1+TIME(0;5;0);7)
for the rest of the cells, rounding up with seven decimal places seems gets the right values even using seconds.

In any case not a bug, developers work hard to avoid rounding issues that comes from the floating calculations with 15 places plus sign limit, it is a hardware limitation, rounds exceeding this limits can give inaccurate results on the last places of the number.
Comment 3 Oliver Brinzing 2019-05-30 06:29:38 UTC
(In reply to m.a.riosv from comment #2)
> This is a rounding issue.

yes, but there is a difference between calculation in LO 6.1 and LO 6.2.
LO 6.1 works (same with excel 2016)
Comment 4 Oliver Brinzing 2019-05-30 06:30:16 UTC
Created attachment 151771 [details]
time_rounding_difference
Comment 5 Oliver Brinzing 2019-05-30 06:30:41 UTC
Created attachment 151772 [details]
time_rounding
Comment 6 Oliver Brinzing 2019-05-30 06:31:03 UTC
Created attachment 151773 [details]
time_rounding.xlsx
Comment 7 Oliver Brinzing 2019-05-31 11:34:37 UTC
seems to have started with:

https://gerrit.libreoffice.org/plugins/gitiles/core/+/e2e47898180e547cad7ccde1e5890385d573e551

Use tools::Time::GetClock() in number formatter for wall clock time

Also handle rounding/scaling better in ImpGetTimeOutput() for the
[] duration formats, of which [HH]:MM:SS(.0000000) is used to edit
time values.

The wall clock change made it necessary to adapt some test cases in
Test::testUserDefinedNumberFormats() where M_PI formatted to
date+time actually is 1900-01-02 03:23:53.60527 with second 53
instead of the previously rounded 54.

Change-Id: I242a6c753a24281e041d3f73af019bdd77c65b37
Reviewed-on: https://gerrit.libreoffice.org/59857
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Jenkins
 3c35d0624c423cd4ccb08340e33de2288f03f4df is the first bad commit
commit 3c35d0624c423cd4ccb08340e33de2288f03f4df
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Fri Aug 31 04:44:34 2018 -0700
    source e2e47898180e547cad7ccde1e5890385d573e551
    source e2e47898180e547cad7ccde1e5890385d573e551

:040000 040000 70cd07a8a294af840f6b5426df0f3f29798e7fa3 06198b34c24a651da6c8455cc818a145b6c23900 M      instdir

$ git bisect log
# bad: [d60ae8383378fcecc7ab077670bf45208a214c71] source e45c30858dec1dd705b9144fab981a3c8819ba96
# good: [b0a56ec98b1368cb5e3e531e0b3f69565af91609] source 3a801799536e6870f2fb111b1cc00b9575a35a39
git bisect start 'master' 'oldest'
# good: [5180a3b7a5dc530ad7ec5bd6e5cefecf85beab7e] source 8bcc4a98d78869d6839821b9747602777f00ebaf
git bisect good 5180a3b7a5dc530ad7ec5bd6e5cefecf85beab7e
# bad: [1473ee9b216ec27c5410a08036aef0a4b857841c] source 93c817971d76ff5020d4210229896a35d357a371
git bisect bad 1473ee9b216ec27c5410a08036aef0a4b857841c
# bad: [0942649607862f8e681eeeb027ead35246eeee6f] source d784612ff788f688eebb851c800228fc01c60470
git bisect bad 0942649607862f8e681eeeb027ead35246eeee6f
# bad: [f8b7df8260b2b21ea776ddda0480d4ecde928aeb] source a9bcbd3dad16c69f1e7ebb52a30611150a49f298
git bisect bad f8b7df8260b2b21ea776ddda0480d4ecde928aeb
# bad: [7f9fa743bc9ed65ad739fe9218babd841e0f22fb] source d587931fba77246db3a2ccc6ab61ca77446d23f4
git bisect bad 7f9fa743bc9ed65ad739fe9218babd841e0f22fb
# good: [e71cd9ebf21b8ce53c5ff534474ea996450c11da] source f4a9ce33415a85d0b86ced3a0bf780f4ec61e25f
git bisect good e71cd9ebf21b8ce53c5ff534474ea996450c11da
# bad: [6bc07af508f6319be42be8a2ea57e2b641c60fc4] source 21f52dc70e0f74adc559375f560dff969b9498de
git bisect bad 6bc07af508f6319be42be8a2ea57e2b641c60fc4
# good: [f1c39124a69c08ee40b71ac0a8a00797dfb3e036] source 3e42e545176a7bfe31fc687ec9ab47db517725eb
git bisect good f1c39124a69c08ee40b71ac0a8a00797dfb3e036
# bad: [344f85171ae55a40653b8b680c20dd2a1c6e8599] source 8eca83829b21bf17f6a83b87df38862eab490b30
git bisect bad 344f85171ae55a40653b8b680c20dd2a1c6e8599
# good: [049b627b927fd4878a0a8137bbf66254886bb23f] source 8eafa504e99bdf946e006527092d1974f18b66cc
git bisect good 049b627b927fd4878a0a8137bbf66254886bb23f
# bad: [00d79a3087f841d4d2acb40b7785c17e510ffaf7] source eb199c1b5e8d6039a1969cc6ddb3d627bedf5bd8
git bisect bad 00d79a3087f841d4d2acb40b7785c17e510ffaf7
# bad: [3c35d0624c423cd4ccb08340e33de2288f03f4df] source e2e47898180e547cad7ccde1e5890385d573e551
git bisect bad 3c35d0624c423cd4ccb08340e33de2288f03f4df
# good: [1b4ea84586792f869e6aeac664772f97e7c0c207] source 370d4c9ba34913076f7a73a2912eb2e0e48ca73c
git bisect good 1b4ea84586792f869e6aeac664772f97e7c0c207
# first bad commit: [3c35d0624c423cd4ccb08340e33de2288f03f4df] source e2e47898180e547cad7ccde1e5890385d573e551
Comment 8 Eike Rathke 2019-05-31 12:12:39 UTC
First off, the expression  =FLOOR(NOW(), "00:05")  is not good practice, as the second parameter to FLOOR() is expected to be a number, not a text string. Whether a string will be automatically converted to a number and if and how depends on the calculation settings under menu Tools -> Options -> Calc -> Formula, Detailed Calculation Settings; the "00:05" may result in either #VALUE! or 0 or 0.00347222222222222, the floating point value of TIME(0;5;0). A correct expression would be =FLOOR(NOW();TIME(0;5;0)), however, whether a significance of 0.00347222222222222 to FLOOR() yields an expected value is up to the reader.. the start value in A1 then is 43616.5763888889

Adding 0.00347222222222222 repeatedly in a series leads to inaccuracy due to cumulated rounding errors, this is in the nature of binary floating point representation, for which precision is not endless, and independent of any date+time formatting.

The difference between 6.2 and earlier versions is that for wall clock time format displays the underlying value is not rounded anymore into the next magnitude, same as a clock does not display 12:35 for a time of 12:34:56; you can see in your attached sample if you format the values in column A as
YYYY-MM-DD HH:MM:SS.00
that starting from around A18 there's the value for example (today,now) 2019-05-31 15:34:59.99

For the expected calculation a working approach would be
=FLOOR(A1+TIME(0;5;0);TIME(0;5;0))
and pull that down.
Comment 9 Eike Rathke 2019-05-31 12:31:11 UTC
We maybe could do further tricks in the interpreter *iff* it is detected that for addition and subtraction operators a (date+)time operand is used and internally round to microseconds or such, but really, that only would masquerade user expectations, and then again in some circumstances the rounding might not be wanted..
Comment 10 Alexandre Rio 2019-07-10 15:54:08 UTC
Thanks for your answer, I'm now using m.a.riosv workaround.
Should I close the issue or leave it open since it may be a regression introduced in LO 6.2?
Comment 11 Oliver Brinzing 2019-08-26 12:29:33 UTC
*** Bug 127143 has been marked as a duplicate of this bug. ***
Comment 12 b. 2021-04-05 01:14:23 UTC Comment hidden (off-topic)
Comment 13 Eike Rathke 2021-04-06 14:43:17 UTC Comment hidden (off-topic)
Comment 14 b. 2021-04-10 14:58:01 UTC
(In reply to Eike Rathke from comment #13)
> (In reply to b. from comment #12)
... 
> And that is completely unrelated and thus off-topic because this bug here is
> not about any fill series but explicit user supplied formula expressions
> accumulating an error. 

yes, SIR!, YES!, 

but with all due respect, and i politely apologize that i have to try to clarify three things: 

1. it's less the 'user defined formula' accumulating deviations, but 'fp-math' and how calc displays timestamps, in three steps: 
   a. the IEEE 754 double value for 5 minutes is already short by ~ 2.22222222E-19 because it's an endless fraction truncated after 53 bits, 
   b. this value looses it's last 24 bits (011100011100011100011100) accounting for appr. 3.2337587468900253E-12 in the addition to a value in the range of today() by crossing 24 bin ranges from -9 to +15, and that without compensation by rounding because the chopped off string starts with a '0', 
   c. thus the really added portion accounts for only ~ 0.0034722222189885, corresponding to a time value of ~00:04:59,9999997206 [hh:mm:ss,0000000000] 
   d. as calc's display for date and time values doesn't round minutes the deviation becomes visible already at very small portions and looks 'boosted' in strings formatted to show minutes but not seconds, 

2. what the OP was talking about is not! his formula being erroneous, but he asks for explanation why it fails in 'dragfill' (which imho is just another interface to construct a fill series), 

3. despite this situation which would also justify other proposals ... what i'd propose is! exactly that: a replacement for the user defined formula avoiding accumulation of errors (exactly it is / needs two combined formulas), 
   a. despite similar could - and imho should - be realized in code it also can! be used in user space as formula in sheet or similar implemented as a macro, it's an exactly targetted solution for the OP problem, imho the simpliest covering the full complexity ... 

thus sorry SIR!, imho you'd shoot fast but a little bit off, please reactivate the suppressed comments. 

@Alexandre Rio: 'but doesn't work with only =NOW() function in first cell.' results from 'now' mostly! not being a value with 0,00000 seconds, thus there is some 'buffer' to compensate the too small represented increment, and thus the problem will show up later after plenty additions, but will ... despite the buffer bridges to a range where the increment get's chopped before a 1 and will be rounded up, then the fail will run in the other direction ... 

removing 'regression' as the change in time stamp handling is intentional,
Comment 15 b. 2021-04-13 08:28:52 UTC
Created attachment 171147 [details]
testsheet for 'fills' with fractional values

hi @all, 

my proposal in c#12 was a little weak, i saw later that it has difficulties e.g. starting from 1970-01-01, 

pls. find attached a new version which can handle that, it needs one special calculated correction value and then reaches ~128 bit precision in doubles (see comments and 'how and why' in the sheet, 

it is 'formula based' and thus userspace (be careful trying it in macros, saw some crazy miscalculations with basic lately), but IMHO similar could / should be implemented in code to get finally rid of all these 'fill' and 'series fails', 

disclaimer: 'work in progress', looks well but not yet extensively tested ... corrections welcome ...
Comment 16 b. 2021-04-15 20:18:32 UTC
i have experimented a bit more, and found that: ... even if you get 'exact' time values for the 5 minute intervals with tricky integer calculations, about half of them suffer 'round down' if their bitstring is truncated before a zero when added to the daily value, e.g. 
~0001110001110 | 00111000 -> ~0001110001110, and the others 'round up' because of the beginning of the truncated part being '1', e.g. 
~0101010101010 | 10101010 -> ~010101010101*1*,  
the rounded up values are 'big enough' to bring the correct result, the rounded down values are not, the 'wall-clock-standard' makes 'just below' -> 59:59 ...? 

a very very small 'protection-add' against such cases (of 1.9E-12, ~0.5 ULP in the range?) which is not harmful in the result but mixes with the 'fp-artefacts', is sufficient to save such cases ... 

maybe you should think about rounding times differently, IEEE 754 provides roundup? or add a small protection-add if the result is in danger to be interpreted as time value according to wall clock standard ... 
happy hacking ...
Comment 17 Alex Sims 2021-10-20 00:45:56 UTC Comment hidden (off-topic)
Comment 18 N.A. Hoijtink 2023-11-12 16:18:56 UTC
I can confirm a similar (same?) issue with off calculations:

if you enter a date+time value in the first row, and then add for e.g. timevalue("00:15:00,000") to the previous cell and the use fill (drag it down) at some point the datetime displayed equals ../../.. xx:59:59

for example enter in the first cell =datevalue(2023;12;3)+timevalue(0,15,0), next cell = "previous cell"+timevalue("00:15:00,000") at some point it results in a unexpected value (1 second less), in my case example at the 25th row.

so to be exact:

A1        03-12-2023 00:00:00,000          (enter some date/time combo in first cell)
A2        =A1+TIMEVALUE("00:15:00,000")    (formula to drag down from cell 2 onwards)  results 03-12-2023 00:15:00
.
.
.
.
A25       =A24+TIMEVALUE("00:15:00,000")                           results 03-12-2023 05:59:59

There seems to be some sort of a precision issue .... and it happens really fast.

My system is a AMD Ryzen 7 5700G running Windows 10 24H1 OS Build 19043.3208 (Dutch), LibreOffice 7.6.0.3 (x86+64) Dutch

Not sure if it is the exact same issue or a more specific date/time issue ....
Comment 19 Commit Notification 2023-11-13 18:38:39 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/007e9e46d936ad7631792188168dffec7051b27f

Resolves: tdf#125580 Limit operator +|- date+time resolution to microseconds

It will be available in 24.2.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.
Comment 20 Commit Notification 2023-11-13 18:38:41 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d646341e5ddc625d9cf69777b4be174aebb43700

Related: tdf#125580 Use tools::Duration constexpr accuracy epsilon values

It will be available in 24.2.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.
Comment 21 Eike Rathke 2023-11-13 18:44:36 UTC
Pending review https://gerrit.libreoffice.org/c/core/+/159393 for 7-6
Comment 22 BogdanB 2023-11-13 18:56:47 UTC
Thanks, Eike for fixing this.

Good in 
Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: d646341e5ddc625d9cf69777b4be174aebb43700
CPU threads: 16; OS: Linux 6.2; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded

Bad in Version: 7.6.2.1 (X86_64) / LibreOffice Community (for comparison reason)
Build ID: 56f7684011345957bbf33a7ee678afaf4d2ba333
CPU threads: 16; OS: Linux 6.2; UI render: default; VCL: gtk3
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded
Comment 23 Commit Notification 2023-11-15 09:55:23 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-6":

https://git.libreoffice.org/core/commit/4582516259812aa1ca8ae95e0bdaae08d0b88f0c

Resolves: tdf#125580 Limit operator +|- date+time resolution to microseconds

It will be available in 7.6.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.
Comment 24 Commit Notification 2023-11-22 20:58:12 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9602f8a9318dd4d3409856e2ae06abe96e72b51b

tdf#125580: sc_subsequent_filters_test4: Add unittest

It will be available in 24.2.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.