GNOME Bugzilla – Bug 622778
Miscalculation in cashflow reports
Last modified: 2018-06-29 22:41:24 UTC
I needed to know where the money came from to pay for my house renovations. I ran a cash flow report. It showed me that a certain amount came from my checking account and a certain amount came from my credit card, etc. I ran a cashflow report on my credit card account. The amount going towards renovations from credit card in my credit card account's cash flow report was superior to the amount coming from credit card towards renovation in my renovation account's cash flow report. These 2 amounts should be equal. I have done manual calculations and the correct amount is the one from the renovations account's cash flow. The miscalculation is coming from a split transaction. I have identified where the problem came from. I have once bought something that was useful for my renovations. I have recorded the transaction in the renovations' account. I have paid for this item in 2 different manners: part of the money came from my credit card and part came from another account (a mortgage account). I have recorded it in a split transaction. What Gnucash did in my credit card's cash flow report is that it took the total amount of the item and put it towards renovation, when only part of this amount was paid for from the credit card. Miscalculations of this exact kind in cash flow reports seem consistent as the exact same thing happened to me before. I had solved the problem by entering 2 transactions instead of one split transaction. However, that did not represent the reality and I considered that to be incorrect accounting. I hope you understand my description of the problem I encountered. Regards, Bruce
Created attachment 168349 [details] A very simple example of split generating such an error. This file shows a salary of 1 000 € paid for half by bank transfer (in cash), for half from a loan granted to the employee. Now make a cash flow report for the sole bank account, from 2010-01-01 to 2010-12-31, in euros. You'll see that the "money out" is 1 000 €, while it should be 500 €.
I confirm the bug. In case of a split, only the cash flow part of the entry should be included into the report. In the rare case a split involves both several credit accounts and several debit accounts and at least one of these 2 sides includes both cash and non-cash account, then the cash part of the entry should be distributed proportionally, but in fact it rather means that, in this case, the user should have made separate entries, because it's unlikely that the cash part of the flow had indeed distributed proportionally, and GnuCash cannot guess where the cash part really came from or where it really went.
*** Bug 338604 has been marked as a duplicate of this bug. ***
This bug is still in version 2.4.10 and is confirmed. Can someone update the status (and version?) please.
It is even in 2.4.11 and what I forgot to mention is that not only the "money out" is wrong, but the non-cash part of the payment is wrongly counted as "money in" the cash account. Andreas Köhler, do you plan to do something for this 3 years old very annoying bug which produces wrong cash reports?
Here is a set of patches that should fix this behaviour. It is a set of three patches, but the first two only do some code restructuring that made my life for handling the code a bit easier. Only the third patch includes the solution. But keep in mind that the third patch cannot(!) be applied stand-alone. I have tried the attached simple example and the described behaviour from Comment 1 does not show up, instead, 500€ is shown as expected. Here the patch descriptions: --------------------------------------------------------- [Bug 622778] Miscalculation in cashflow reports - Step 01 Restructure code to achieve following blocks - options generator - global objects - cash flow calculator - document renderer Reason: Preparation for better change tracability. No functional change of code. --------------------------------------------------------- [Bug 622778] Miscalculation in cashflow reports - Step 02 - functional change within the cash flow calculation: Instead of using a recursive call to (define (calc-money-in-out-internal ...)) to loop through the list of selected accounts a for-each loop is introduced. - functional correction: As the cash flow result data has been turned to global objects with the previous patch, these objects need to be reset before the call to the cash flow calculator. This is now added in the document renderer. - further comments introduced This is used to help identifying the function change that will be introduced with the next patch for fixing 622778. --------------------------------------------------------- Bug 622778] Miscalculation in cashflow reports - Step 03 Introduce solution and some minor modifications to save some function calls. Solution: - Save the current split value - Calculate the parent total transaction value - Calculate the ratio of the split to the transaction value - only consider splits of opposite sign for the flow colletion - only consider the split value multiplied with the calculated ratio of those splits with opposite sign
Created attachment 241845 [details] [review] patch file 1 to be applied on cash-flow.scm
Created attachment 241847 [details] [review] patch file 2 to be applied on cash-flow.scm
Created attachment 241848 [details] [review] patch file 3 to be applied on cash-flow.scm
This bug should be moved to NEW.
As the patch files were developed on GnuCash 2.4.10, I just checked whether there is a difference between the original cash-flow.scm in GnuCash 2.4.10 and 2.5.0. This does not seem to be the case, meaning that this patches should be safe to be applied on top of GnuCash 2.4.10 or later.
Comment on attachment 241845 [details] [review] patch file 1 to be applied on cash-flow.scm Thank you for your patch. I haven't looked in depth yet, but I have to make the same remark as I made for your patch in bug 589865 : Please don't change the name or report GUID in patches intended to fix a report. Only use new names and GUIDs if you really want to add a new report. I understand that changing the name/guid can be handy while testing, but if you have to, please do so locally only (for example in a separate local git commit or don't commit the change at all by using git add -i). I'll test the patches soon.
On a general note: your reorganising of the code makes it more or less impossible to judge what key parts have changed. I'm ok with this. I'm sure the report is much easier to maintain now. However given these big changes the risk for subtle new bugs gets bigger as well. As a result, I don't think it should be applied to the stable 2.4 series, but rather to the 2.5 series only.
Comment on attachment 241845 [details] [review] patch file 1 to be applied on cash-flow.scm I have reverted the two unwanted changes and applied your patch as r22968 (trunk). Thank you very much ! As mentioned, I won't backport this to 2.4.
Comment on attachment 241847 [details] [review] patch file 2 to be applied on cash-flow.scm I have applied your patch as r22969 (trunk). Thank you very much ! As mentioned, I won't backport this to 2.4.
Comment on attachment 241848 [details] [review] patch file 3 to be applied on cash-flow.scm I have applied your patch as r22970 (trunk). Thank you very much ! As mentioned, I won't backport this to 2.4.
Hmm, the revision numbers are off by one. The correct numbers are: Patch 1: 22969 Patch 2: 22970 Patch 3: 22971
These patches probably fix the problem, but they produce ugly output. For example in a cash flow report that I have open an amount that should be $3383.00 is "$3,383 + 0/1". This means the total money out is "$19,092 + 23/100" and the difference is "-$19,092 - 23/100". These numbers seem to be correct, but they aren't very user friendly. This is probably happening because of the new code that tries to figure out what proportion of a transaction's value is the result of a particular split, but I haven't figured out how to fix it.
I figured out a fix that I think is ok and checked it in as r22976. When computing the fraction of a transaction's value due to a particular split, it now rounds the result to the transaction's currency's fraction.
Thanks to both, Geert and Mike, for fixing the flaws in my patches. Regarding name/guid - sorry about that, I simply forgot to remove it, absolutely clear to me that this should not be kept in a patch Regarding ugly output - good that you tested it, I was not coming across it. Can you add the patch for this to this bug, so I can merge it into my setup? I would also like to apply it to my 2.4.10 installation. Thanks again!
Created attachment 243956 [details] [review] Patch to round value properly Patch for SVN r22876 that fixes the rounding problem when calculating the proportion of the transaction due to a specific split.
Thank you for your work. If I understand well, it is still not in the stable release. Since it is a serious bug (producing wrong cash flow reports), what about releasing a new version for it?
(In reply to comment #22) > Thank you for your work. If I understand well, it is still not in the stable > release. Since it is a serious bug (producing wrong cash flow reports), what > about releasing a new version for it? See my comment 15 for a motivation not to backport this to the 2.4 series. The "new version" will be 2.6, which we are in the process of stabilizing for release (currently expected by the end of the year).
(In reply to comment #21) > Created an attachment (id=243956) [details] [review] > Patch to round value properly > > Patch for SVN r22876 that fixes the rounding problem when calculating the > proportion of the transaction due to a specific split. Mike, Did this patch ever get committed ? I notice it's status is "none" and since it's attached to a closed bug, it won't appear in the list of unreviewed patches.
Yes, I committed it first (see comment #19) then created the patch as described in comment #21. There is a typo in comment #21, the commit is r22976 as given in comment #19. I'll change the status of the patch.
Review of attachment 243956 [details] [review]: Applied before the patch was created.
(In reply to comment #25) > Yes, I committed it first (see comment #19) then created the patch as described > in comment #21. There is a typo in comment #21, the commit is r22976 as given > in comment #19. I'll change the status of the patch. Ah, I didn't see that. It's safe to forget about this bug then :)
After a long discussion in bug 722140 and a related mailing list thread, I have come to the conclusion this report doesn't represent a bug in the cash flow report but rather a misunderstanding of how the cash flow report should be used. So I will revert the patch for this report for the next gnucash release in September. If you want to add anything to the discussion, please do so on bug 722140.
Mike: I have reverted the patches attached to this bug as a result of the discussion on bug 722140. However you later have also applied yet another patch to exclude the trading accounts from the cash flow. This patch also built on Carsten's 3rd patch so I had to revert it as well. With the discussion on bug 722140 in mind, I'm not too sure how we really should handle trading accounts. Your patch hid them from the cash flow report unconditionally. Is that really what we want ? Or are there use cases where one actually wants to see the cash flows to and from a trading account ? Perhaps we should bring this discussion to bug 722140 as well...
I can't think of any reason to including trading accounts in cash flow reports. Excluding them unconditionally seems ok, but if you think it should be an option that would be fine with me. Perhaps there is already some way to exclude them. If so that would be ok. I'm in London right now and don't have access to my development environment. If necessary I'll take a look at this when I get home on August 10. I can create a new patch to handle trading accounts however people think is appropriate at that time.
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=622778. Please update any external references or bookmarks.