Bug 81757 - Calculations order do not follow cell dependencies
Summary: Calculations order do not follow cell dependencies
Status: CLOSED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.1.6.2 release
Hardware: All All
: medium major
Assignee: Not Assigned
URL:
Whiteboard: target:7.2.0 target:7.1.5
Keywords: bibisected, bisected
: 129588 (view as bug list)
Depends on:
Blocks: Calculate
  Show dependency treegraph
 
Reported: 2014-07-25 18:45 UTC by Ryszard
Modified: 2021-05-26 09:25 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
file with problem (31.74 KB, application/vnd.oasis.opendocument.spreadsheet)
2014-07-25 18:45 UTC, Ryszard
Details
2 versions of this file (52.34 KB, application/zip)
2014-07-27 02:24 UTC, Ryszard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ryszard 2014-07-25 18:45:44 UTC
Created attachment 103462 [details]
file with problem

The attached spreadsheet was created a while ago, from this what I can check the last version of libreoffice that was working as expected was 4.0.
I have check this on Windows with 4.1.6 and linux 4.2.5 release versions.
It looks like cell recalculation are based on position not dependencies.
In attached file sheet1 is used to calculate bend allowance data for sheet metal flat layout calculation, it contains working macro and formulas.
We have just noticed different results in cells I8, F28, in different program versions. It looks like the value of cell I8 is calculated before I4.So the value of I8 is based on previous value of I4. If I copy this formula to call I6 the value is updating correct in I6, but not in I8.
Comment 1 ign_christian 2014-07-26 06:01:51 UTC
Hi.. Please provide step by step procedure so others can easily test & triage the issue, for example: https://bugs.freedesktop.org/show_bug.cgi?id=73678#c0
Comment 2 Joel Madero 2014-07-26 21:54:30 UTC
I can't reproduce this - I get the same values on 3.3 (first release of LibreOffice, 4.2.6.2 release, and 4.4 master)

Marking as NEEDINFO

@Ryszard - please provide the #'s that you expect and what version of LibreOffice you see these numbers. I'm afraid the document itself might have been opened and saved and now the formulas themselves are messed up (so even on 4.0 or 3.6) you'll see the values (and then the test document isn't useful for confirming)

If you can let us know what # you expect and reproducible steps please set to UNCONFIRMED. Thanks
Comment 3 Ryszard 2014-07-27 02:24:33 UTC
Created attachment 103529 [details]
2 versions of this file
Comment 4 Ryszard 2014-07-27 03:01:43 UTC
I have attached zip file with 2 versions of this spreadsheet also with shown columns containing K factors used to calculate bend allowance in sheet metal fabricating, one older with less formulas that seems to works correctly even in 4.2.5.2 version of Libreoffice. The newer file contains more formulas that allow enter dimensions of the part and get size of flat layout (cells E20:E28, and F28). There are also formulas that allow me to find K factor that I'm using in SolidWorks to change flat layout for parts given by customers with different radii that we are using to manufacture these parts (cells J12:J14 and N13, N14, O13)
At this moment I found something wrong in calculation of cells I8 that it should be equal to I4*F8 and automatically F28 that is using I8.
How it should work.
Based on bend type (drop down box), thickness of the material radius of bend and bend angle, macro function is calculating value of cell F7(bend allowance), this value is used to calculate F8, up to this everything is fine. Now the I4 cell holds number of bends in part (at this moment 1), based on that the cells I7, I8 should be equal to bend allowance and bend deduction for part. Since there is only one bend the values of I7 should be same as F7 (and is), value of I8 should be same as F7. It is when you open the file, but if you change item in drop down box the value of cell F8 will change, but cell I8 will not.
If you select another item in the drop down box, the value of I7 cell will be the previous value of F8. To get correct answer you will have to select item from drop down box twice, or reenter same value to the one of cells.
The more strange thing will happen if you will change the radius value let say from 0.010 (value on file open) to 0.0285 (I have used that), somehow the cell I8 shows 0.1222 that I can not relate to anything in the spread sheet. Entering again 0.0285 to the radius will make calculations correct.
Comment 5 Ryszard 2014-07-28 14:51:45 UTC
There is one more thing that changed since LibreOffice 4.0 , the iteration option. This spread sheet worked correctly with iteration option unchecked, now there are Err:522 in some cells if this option is not set.
Comment 6 Joel Madero 2014-07-28 15:13:50 UTC
Please report a separate bug report with a test case and reproducible steps
Comment 7 Ryszard 2014-07-28 16:30:29 UTC
What different bug?
We are still talking about the same bug, I was just trying to give you more clues about it, so you may trace it better.
I was playing with the macros to figure out where is the problem.
So the BENDALLOWANCE macro is also changing cell J12, this cell is not used in any other formulas it is only information, but somehow now it is triggering recalculations, so that is why iteration is needed, and the other values are not calculated correctly. I have commented out this line, and everything worked fine.
Than I have changed the cell J12 to be calculated as result of macro BENDFACTOR same way the F7 is for BENDALLOWANCE. Everything works fine except, it brought question about extremely strange behavior(and I'm not reporting this as a bug).
Is the new version (4.2.5.2 ) of libreoffice caching opened files. When I have saved this spreadsheet using version 4.0, opened file in 4.2 version was still same old bad file, I had to save it under different name using 4.2 version to get it back correctly after reopening in 4.2. The 4.0 version was showing all changes in place.
Comment 8 Buovjaga 2014-11-27 06:58:09 UTC
Should be UNCONFIRMED apparently.
Comment 9 raal 2014-11-28 18:08:24 UTC
When I open file bendAllowance.ods in LO 4.5.0.0.alpha0+ and 4.3.6.0.0+ I get error message :BASIC runtime error. Object variable not set


Setting as new - when I change combobox item, then I need hard recalculate CTRL+SHIFT+F9 to get correct calculation.

Version: 4.3.6.0.0+
Build ID: 9e57326acebde972df22ea4368b5ce4822d51330
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:libreoffice-4-3, Time: 2014-11-27_12:21:59
Comment 10 Ryszard 2014-11-30 21:06:00 UTC
In one point LO must change the trigger for recalculating sheet, and now recalculations are also triggered by change made from custom macros.
In this case I have found out the problem is that cell J12 is changed from macro function. It looks like this is causing auto recalculation, of course this leads to infinite loop so "Iterations" option must be set, and values of depending cells might be incorrect since macro functions are never finished. If I go to macro editor I'm getting error variable not optional with highlighted line " If ((insRadius/thickness) <= 1 ) Then". This error is only shown in BASIC editor.

To force calculation you may use hard recalculate, or enter same data or set same option again.

commenting lines oSheet.getCellRangeByName("J12").value = kFactor(selItem
 in macro functions and making value of cell J12=BENDFACTOR(radius,angle,thickness) will make everything back to normal, also "Iterations" option may be unset.
I do not know if this LO behavior was intentional, but for me it is quite critical bug. Many people may not realize about values not being updated correctly in their spreadsheets, OO 4.1.1 is working as expected.
Comment 11 raal 2015-01-18 11:13:15 UTC
*** Bug 79770 has been marked as a duplicate of this bug. ***
Comment 12 QA Administrators 2016-02-21 08:35:20 UTC Comment hidden (obsolete)
Comment 13 Miguel 2016-02-21 20:30:07 UTC
>    Test to see if the bug is still present on a currently supported version
> of LibreOffice 
>    (5.0.5 or 5.1.0)  https://www.libreoffice.org/download/

I am in copy for this bug because I had reported another bug which was considered to be a duplicate of this one. The other bug (https://bugs.documentfoundation.org/show_bug.cgi?id=79770 - bug 79770) does not exist anymore to me, so I remove myself from the copy list. I am still not so sure that it was really a duplicate to I won't change the status of this bug.

Tested on 5.1.0.3, running in Linux Ubuntu, 32 bit version.
Comment 14 QA Administrators 2017-03-06 15:13:18 UTC Comment hidden (obsolete)
Comment 15 m_a_riosv 2018-01-12 16:05:27 UTC
A hard-recalc [Ctrl+Shift+F9] gives the correct result on bendAllowance.ods, the same than the bendAllowance-old.ods
Comment 16 QA Administrators 2019-01-13 03:59:11 UTC Comment hidden (obsolete)
Comment 17 b. 2019-12-23 15:23:06 UTC
>>> thats the simple thing that shouldn't be that way, a cell with a formula should evaluate to the same value regardless where you move (not copy) it into the sheet. <<<

it has taken some time to go to the construct and dependencies ... 

working with fresh - not *-old - version of the zip-file from comment #3, and testing 'If you select another item in the drop down box, the value of I7 cell will be the previous value of F8. To get correct answer you will have to select item from drop down box twice, or reenter same value to the one of cells.' from comment #4: 

boils down to: 

straightforward calculation of F7,F8, I7 and I8 is! possible if you respect the following dependencies: 

F7 -> F8, F7 -> I7, F8 -> I8

(F7 itself may contain problems and circularity or whatever from performing the macro, but once it's calculated the other three cells are simple) 

obviously that isn't performed by calc in versions 4.1.6.2 to 6.5.0.0.a0, and F8, I7, I8 show err:523 without iteration, (slightly different between versions) 

with iteration enabled the err: messages disappear, but - looks like - I7 and I8 are calculated in the first round, acc. to the value of F7 and F8 at that time, and F7 and F8 undergo a second cycle setting them to new values what is not applied for a recalculation of I7 and I8, 

simply the order for the calculation is wrongly set? if you move! (ctrl-x - ctrl-v) I7:I8 to B7:B8 the correct calculation is performed! on the first select in the dropdown box for material. 

thats the simple thing that shouldn't be that way, a cell with a formula should evaluate to the same value regardless where you move (not copy) it into the sheet. recalculation must first follow dependencies, then the order of cells on the sheet. 

https://bugs.documentfoundation.org/show_bug.cgi?id=129199 looks very similar to this problem. 

until that's solved other more complex problems in this sheet couldn't be analyzed. 

(my assumption: calculation of F7 is 'set back' due to the neccessity to run the macro, dependencie analysis is not! performed afterwards.) 

still buggy with: 

Version: 6.4.0.1 (x64)
Build ID: 1b6477b31f0334bd8620a96f0aeeb449b587be9f
CPU threads: 8; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: GL; VCL: win; 
Locale: de-DE (de_DE); UI-Language: en-US
Calc: CL

and 6.5.0.0.a0
Comment 18 b. 2020-02-19 01:46:38 UTC
! improvement found, danger of passing away, i'd appreciate a quick recheck !

quick: check this bug with ver. 6.2.7.1 winx64 or 6.2.8.2 winx64, for me it looks gone. 

danger: it is showing up again in subsequent versions, 
some tested: 6.5.0.0.a0, 6.4.0.1, 6.4.0.3, 
and even 
7.0.0.0.alpha0+ (x64) Build ID: 07b1159b79135857dd9a450c3bb9ae0a944ebcf9
CPU threads: 8; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: default; VCL: win; Locale: de-DE (de_DE); UI-Language: en-US Calc: 
all failed, 

thus 'regression'? maybe easier to catch now than later. 

sorry, for a correct detailed description this post will become long, imho it's important to read and to check! 

steps to reproduce: 


A): to see the fail: 

1. use one of the buggy versions mentioned above (or to test any other), 

2. load attachement 'file with problem': 

https://bugs.documentfoundation.org/attachment.cgi?id=103462

3. allow macros, 

4. enable 'edit document', 

5. use the dropdown box spanning from D6 to F6 to select a material, 

5a. in some situations the box is in 'design mode' and cant't be used for selection, that can be corrected by 'view - toolbars - form controls' and then deselct 'design mode' in the (often floating) toolbox, 

6. after changing the selected material to another metal observe I7 and I8 showing wrong - outdated - values, 

(acc. to the formula they should show the same values as F7 and F8 while I4 is '1', when the error kicks in they are 'one cycle off' as described in other comments for this bug)

7. note that the displayed values in I7 and I8 only appear to be correct if you select the same material again, 

(it only looks! that way because new and former calculation produce the same result), 

8. observe that after changing to another materal again the values in I7 and I8 are wrong again, 


B): to see the fix: 

1. use ver. 6.2.7.1 winx64 or 6.2.8.2 winx64, 

... proceed as above ... 

6. in this step after changing the selected material to another metal observe instant reaction to correct values in I7 and I8, 

7. play around as you like, I7 and I8 follow correctly, 

--- i can't say! --- 

- if this better behaviour is intentional (a patch?), by chance (side effect of another patch?) or just randomly, depending on my mood or room temperature, 

- if this behavior is stable or will disappear tomorrow, 

(did some criss-cross checks and rechecks, looks stable at the moment)

- if this behaviour is a 'full solution' or just affects this one flavour of something more general? 

thus i'd like a recheck by other testers, and if i was lucky to spot a nugget that someone invests the effort to nail it down and port it to the actual releases ... 

reg. 



b. 

P.S. fault looks independent of the settings for parallelization ('threaded' or openCL) in all versions, 

P.S.II. the 'problem' with J12 and the need of 'iterations' from c#7 (which have not been neccessary in older versions?) are still present ...  

P.S.III as the better behaviour for this bug kicked in with ver. 6.2, while bug #129199 with somewhat similar description newly appeared in those versions, i assume them being 'related', unfortunately we have both problems active in 6.4 versions :-( ... thus it's not a simple toggle in recalc ordering?
Comment 19 Buovjaga 2020-04-29 18:38:02 UTC
*** Bug 129588 has been marked as a duplicate of this bug. ***
Comment 20 b. 2020-10-21 09:16:23 UTC
issue doesn't show up in 6.2.7.1, 6.2.8.2, 
but in 6.1.6.3 and plenty versions after 6.2, up to 7.1, 
thus i think it's fixed somewhere in the 6.2 branch, may be 'en passant' and thus not backported? might someone with bibisect capa check which patch changed it ... or if i'm lost in the jungle of options? thanks in advance ... 

it looks as in older versions only I8 is lagging, in newer I7 and I8 ...
Comment 21 Buovjaga 2020-10-29 18:47:33 UTC
(In reply to b. from comment #20)
> issue doesn't show up in 6.2.7.1, 6.2.8.2, 
> but in 6.1.6.3 and plenty versions after 6.2, up to 7.1, 
> thus i think it's fixed somewhere in the 6.2 branch, may be 'en passant' and
> thus not backported? might someone with bibisect capa check which patch
> changed it ... or if i'm lost in the jungle of options? thanks in advance
> ... 
> 
> it looks as in older versions only I8 is lagging, in newer I7 and I8 ...

I bibisected the change in with win32-6.2 repo to https://git.libreoffice.org/core/commit/00b3007976f60bdf65fbe69e3f4f1613b477a059
Avoid side effects of additional Interpret() during iteration
Comment 22 Timur 2021-05-24 11:38:23 UTC
I didn't check, but setting bibisected per comment 21 and adding Eike.
Comment 23 Eike Rathke 2021-05-25 19:25:56 UTC
The moment you observed Err:522 you should had gotten suspicious.. the culprit is the macro that while it has a CalculateHard running that calculates BENDALLOWANCE() that modifies the cell content of J12, which for the current view area triggers a find for changed values and calculates formula cells that are set "dirty" (i.e. to be recalculated), which then for F8 wants to obtain the result of F7 that currently is already running, hence the circular reference.

0. best do not try such dirty tricks as modifying cell content in a formula cell's macro function. It may work or may not and may have side effects you aren't aware of. As seen.

1. take such Err:522 circular reference error serious if you do such thing and not expect it (i.e. the formula cells and the cell references they use are not circular); just switching on iterations to workaround and suppress it may just hide the cause and fool you with an unexpected result. As seen.

2. if you feel brave and want to do such nasty things anyway then switch off all updates while the recalc executes to not have the "update the screen from dirty values and thus recalculate" triggered, i.e. in your sub recalcul lock and unlock the controllers with

dim component as object
component = ThisComponent
component.lockControllers()
... CalculateHard ...
component.unlockControllers()

which in this case works.

However, the better solution would be to not have the macro functions fiddle with cell content at all but rather calculate J12 properly with another function. Also, it would be better to not have the listboxes trigger a CalculateHard on selection, but rather write the selection to a cell (or use Data Validity instead of a form listbox) and use that cell as parameter to functions. You wouldn't need any manual recalc then.
Comment 24 b. 2021-05-25 19:54:50 UTC
will try to 'go through' and understand later, for the moment 'thank you very much @erAck', from such analysis and lessons people can learn!
Comment 25 Eike Rathke 2021-05-25 20:13:10 UTC
Fwiw, having the form listbox selection events call recalcul() and also the recalcul call in Main may lead to them being executed simultaneously nested upon loading the document as form events are fired asynchronously, which makes things even worse.
Comment 26 Commit Notification 2021-05-25 21:17:01 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/807f3508c9319d7f94820ea1282e306594418fce

Related: tdf#81757 Do not even try to re-enter ScDocShell::Do*Recalc()

It will be available in 7.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 27 Commit Notification 2021-05-26 06:57:15 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

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

Related: tdf#81757 Do not even try to re-enter ScDocShell::Do*Recalc()

It will be available in 7.1.5.

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 28 Eike Rathke 2021-05-26 09:19:36 UTC
Closing. We should have a FOOTSHOT status..