Created attachment 373208 [details] GnuCash stdout when running the report All, my balance sheet report is broken and displays "Report error. An error occurred while running the report." This happens when - Show exchange rate is ticked in report options - "Average cost" price source - A foreign currency account exists, with a zero balance (after some transactions) - Trading accounts enabled See Gnucash stdout attached. This is probably related to #775368 Thanks! - Stefan
Created attachment 373209 [details] Minimal sample to reproduce the bug See attached file as a minimal example. To reproduce, run the balance sheet report with - base currency EUR - Show exchange rate enabled - Price source "Average cost"
There's something deep inside commodity-utilities.scm which is causing total-CNY to be amt=0 val=0 hence they can't calculate price. if we force the gnc:get-exchange-cost-totals to add absolute values, the report doesn't crash; however I'd wish confirmation that the prices otherwise remain correct? (for jralls - this causes test-commodity-utilities.scm to fail) (part of gnc:get-exchange-cost-totals) modified gnucash/report/report-system/commodity-utilities.scm (for-each (lambda (a) (if (not (eq? (xaccAccountGetType (xaccSplitGetAccount a)) ACCT-TYPE-TRADING)) (let* ((transaction-comm (xaccTransGetCurrency (xaccSplitGetParent a))) (account-comm (xaccAccountGetCommodity (xaccSplitGetAccount a))) - (share-amount (xaccSplitGetAmount a)) - (value-amount (xaccSplitGetValue a)) + (share-amount (abs (xaccSplitGetAmount a))) + (value-amount (abs (xaccSplitGetValue a))) (comm-list (or (assoc transaction-comm sumlist) (assoc account-comm sumlist))))
Those values probably are correct. In fact, the calculation takes into account buy and sell transactions - if they cancel out as in this case, the algorithm is not able to determine a price. My understanding is, that this is a consequence of the calculation method and is in fact expected: When buy and sell transaction cancel out, the balance for the account using that currency/stock is zero (*) => translating to zero value in base currency no matter what the actual price is. => Although the individual share price cannot be determined, the balance report should be created. Affected accounts will have zero amount in any currency, independent from the share price (being mathematically correct, we have to assume a finite value for the undetermined share price, which is however a valid assumption in most cases ;-) ) Summarizing: There is no need to change the calculation method; please check whether it is possible to show the balance sheet and just do not display the exchange rate for this particular currency. Note: I just checked with version 2.6.17, using the commodity-utilities.scm revision 5803c14 [1] - works exactly like that. Thanks a lot! Cheers - Stefan ---- (*) NB: In the unlikely event of having two accounts of the same commodity with exactly opposite values (x and -x), they would also cancel out and the algorithm is not able to determine the price. This would lead to the problem, that each accounts value is undetermined. In the balance sheet, this does not matter for the totals, as the sum over both accounts is zero again, independent from the share price - but the individual values are undetermined. => This is a somewhat theoretical discussion now, but it would be resolved by a 'per-account average cost' method as proposed in bug #775368 [1] https://raw.githubusercontent.com/Gnucash/gnucash/5803c141c18a6ff75a7f49a9142f834a596857f5/src/report/report-system/commodity-utilities.scm
Ok I gathered that the amount 0 and value 0 were fine. I would be keen to find an easy algorithm for the per-account average-cost -- I can try make it work... (and I can guarantee my code will be easier to follow than current!) Here's another attempt -- this one will cause the RMB/EUR price to be 0. modified gnucash/report/report-system/commodity-utilities.scm @@ -697 +697 @@ construct with gnc:make-gnc-monetary and gnc:monetary->string instead.") (map (lambda (e) (list (car e) - (if (zero? ((caadr e) 'total #f)) #f + (if (zero? ((caadr e) 'total #f)) 0 (abs (gnc-numeric-div ((cdadr e) 'total #f) ((caadr e) 'total #f) GNC-DENOM-AUTO (logior (GNC-DENOM-SIGFIGS 8) GNC-RND-ROUND))))))
Wow, that was quick. From the very first glance this seems to do the job - thanks!
Let us be clear, please. The problem is in the report not the tx. Someone made a presumption about something many years ago when the first balance sheet report was written and everyone has been too scared to look at it ever since. What Steffan is reporting is too close to my own reports to be co-incidental. Steffan: take a look at the end of #797046 you'll find I am reporting a similar problem and have detailed the inputs that can make the Balance Sheet fail in some detail. Christopher: there is no absolute "average cost" in accounting terms, the best we can do is the mathematics, at the moment the gnc reports aren't satisfying anyone, my advice is go for the mathematically correct answer for now. Christopher: I do understand costing, I have passed exams (I shouldn't have to say that) I just don't understand the sums gnc uses in it's reports. If I could read scheme I'd be able to tell you where the problem was but it is opaque to me. I can get the right numbers using sql and plain-text accounting. What is going on?
Wm, bug 797046 is about using a pricedb price source (either latest price or nearest in time) and this report is about using the "average price" price source which calculates a nominal price from the actual transactions. Yes, it's not a very good name and has caused some confusion in the past. Steffan proposed in a comment to bug 775368 today that we rename it to Sellinger's term "adjusted cost base" (aside, shouldn't that be "adjusted cost basis"?). That seems clearer to me, does it to you? Naming aside, isn't that what you mean by "mathematically correct"? To help you understand how the "average cost" calculation works and why it was written that way I suggest the mailing list thread starting at https://lists.gnucash.org/pipermail/gnucash-devel/2008-July/023297.html.
(In reply to Christopher Lam from comment #4) > I would be keen to find an easy algorithm for the per-account average-cost > -- I can try make it work... (and I can guarantee my code will be easier to > follow than current!) I'd be happy if you were willing to give it a try - I'll do my best to explain what I think the calculation should do. Unfortunately, I can't help much with the scheme code itself. Please let us follow-up on that in bug 797138. For the original report, I' think this bug can be closed if Christopher's change finds its way into maint. - Stefan
I'll leave it to jralls to decide if this fix is the right answer here. I'm still trying to figure out commodity-utilities!
(In reply to John Ralls from comment #7) I'm presuming you meant "average cost" not "average price" above as it makes more sense. There is certainly a naming problem, "average" is not well understood, if one is producing a set of accounts you get a chance to add notes saying how you calculated things like "average", I guess most gnc users just play with the options until they get the answer that is most convenient. I think the addition of extra report costing or pricing options without a review of the extant ones would be a mistake, I don't think a proliferation helps most users unless they are well documented and even then some of the choices will by definition make no technical difference to many, "why are you adding options I don't understand" one day and "why isn't the costing / pricing option I want there" the next. I read the thread you suggested. It includes mathematically correct sums, the problem with accounting is that it includes time. I am not expecting gnc to include massive inflation or deflation, this is just a thought example for 2 decimal economies: if you bought something in Venezuela 1 month ago the cost in whatever currency you choose will be a mess today, probably the more you paid for it the less it would be worth now, this screws with our conventional view of value *BUT* I think it also provides an economic short term accounting view of cost vs value. Long term average costing is more complex, is something owned 20 years ago worth more or less now on a cost basis (I think we tend to value things more the longer we have them, that doesn't mean the first LP I bought is actually valuable, etc). I tend to use [number of commodity] * [nearest price in time] for money values because the money sums work. Different jurisdictions have different tax rules, if not for that everyone should be using FIFO.
Wm, Yes, sorry, "average cost". The significant difference between the "average cost" and "nearest in time" price sources is that the former calculates the price from transaction data (your "mathematically correct" approach) while "nearest in time" retrieves from the pricedb the price nearest to the report date (or to the datum date for time-series reports like the networth charts). If you use "nearest in time" you're very likely to get an "Unrealized Gain/Loss" entry in a balance sheet report and very unlikely to get a trial balance report to balance. Your point about too many options is valid, but there are valid use-cases for both per-account and global pricing. I'm not sure that there's a use-case for the current "weighted average" option--in part because it doesn't do what the name suggests and I have to go look at the code to remember what it does do. That's obviously a candidate for deletion. "Most recent" is another: What's the use case for using today's prices on a report dated at last year's end?
John, I didn't explain that well, I meant using [number of commodity] as the absolute because that is what a person or entity has, the value is variable and depends on a price at a time and (in the case of my wonky TB) depends on what gnc thinks I have. I shouldn't have said "nearest in time" as it seems to have confused things, I do understand the pricedb. Aside: there are very few tx in my stream that cause the TB to go wrong, I have pointed the "bad" tx out and worked out why they go wrong. It is OK for you to say "not worth fixing". They aren't stopping me doing anything now I know what the error is, it was not knowing / understanding that was bugging me. If someone else comes across the same problem they are almost certainly Trumped in the genitals. The problem with a lot of the costing / pricing / valuing is that it is borrowed from old code so someone like myself (I can do the maths and the accounting) isn't sure which are good or not. Surely it is reasonable they should be able too be expressed in spreadsheet terms so they can be reviewed? Then we can have a look, get rid of the unused and decide on some that might be more useful.
Ah, OK. The problem with that approach is that in order to determine if your books are in balance the accounting equation must be satisfied in every one of the commodities that you use: Assets in say AAPL must equal liabilities + equity in AAPL and the same for IBM, USD, GBP, EUR, whatever. I doubt that your book is like that. The alternative is to convert everything to a single commodity in which to calculate the accounting equation, and for most users the commodity that makes sense is their "home" currency; USD for me, GBP for you. That's accomplished by choosing a price to do the conversion and using [number of commodity] * [chosen price] to value the account balance.
For the trial balance report we're also concerned that another equation balances: amount spent buying asset + realized gains = book value of asset + proceeds from sale of asset For example, if I buy 200 shares of IBM for $50 each and sell 100 of them later for $60 each, $10000 (cost) + $1000 (cap gain) = $5000 (book value) + $6000 (proceeds). Obviously less simple in a case where there are many trades in and out, especially if they occur in more than one account. The so-called "average cost" calculation, sum(abs(amount))/sum(abs(shares)), produces a price that can be used to calculate the cost, proceeds, and book value that will balance the cap gain and is arithmetically equivalent to pricing each transaction individually and summing the results. The latter is impractical in the use-case of a company doing business in multiple currencies where the transactions are spread over a variety of asset, income, expense, and liability accounts.
(In reply to John Ralls from comment #13) > Ah, OK. > > The problem with that approach is that in order to determine if your books > are in balance the accounting equation must be satisfied in every one of the > commodities that you use: Assets in say AAPL must equal liabilities + equity > in AAPL and the same for IBM, USD, GBP, EUR, whatever. No, that isn't how it works; honestly, John, that isn't what is expected. > I doubt that your > book is like that. JohnR, I don't wish to insult you but I am saying you don't understand. Concentrate on the round trip file I mentioned regarding the TB if you want an example. gnc was left not knowing what the value of some things were. I showed my workings. > The alternative is to convert everything to a single commodity in which to > calculate the accounting equation, and for most users the commodity that > makes sense is their "home" currency; USD for me, GBP for you. That is suitable for people that only occasionally use anything other than "home" currency and no commodities. I am aware of the "book currency" proposal. The problem is that the moment someone makes an investment (unless it is a same currency bank deposit or similar) they are no longer using "home" currency, they are using a commodity, a tradeable thing. > That's > accomplished by choosing a price to do the conversion and using [number of > commodity] * [chosen price] to value the account balance. For people that never travel, never have a 401k (or whatever a pension account is called in the USA), never invest in anything, etc and generally aren't interested in whether their mortgage is worth anything since they got screwed in the last fallout, that is fine, I agree.
Sorry, I've lost track of the "round trip file". Where was that?
(In reply to John Ralls from comment #14) > For the trial balance report we're also concerned that another equation > balances: > > amount spent buying asset + realized gains = book value of asset + proceeds > from sale of asset that isn't a basic TB equation I'm familiar with if you have a 101 accounting ref for that I'd be interested in seeing it (extended TB is different, gnc's TB worksheet is good, unfortunately it depends on wrong sums in the basic parts of the report) > For example, if I buy 200 shares of IBM for $50 each and sell 100 of them > later for $60 each, > > $10000 (cost) + $1000 (cap gain) = $5000 (book value) + $6000 (proceeds). Ummm, not quite 2010-10-10 cash -10000 2010-10-10 IBM +200 @ $50 ~ +10000 2010-11-10 IBM -100 @ $60 ~ -6000 2010-11-10 cash +6000 capital gain is probably income and a tax issue specific to a jurisdiction the left over stuff value changes every time someone else buys or sells a share and is an asset <-- that's what balance sheets are for The difference between what you bought and what you sold is what the Income Statement or P+L is for. TB's do actually serve a purpose :) > Obviously less simple in a case where there are many trades in and out, yes > especially if they occur in more than one account. yes > The so-called "average > cost" calculation, sum(abs(amount))/sum(abs(shares)), produces a price that > can be used to calculate the cost, proceeds, and book value that will > balance the cap gain and is arithmetically equivalent to pricing each > transaction individually and summing the results. and has fuck all to with a trial balance! the trial balance is for double checking stuff, you want accuracy not pretty. > The latter is impractical > in the use-case of a company doing business in multiple currencies where the > transactions are spread over a variety of asset, income, expense, and > liability accounts. It was useless to me when I was wondering where the missing money was, it turned out to be a currency round trip on a single date that was throwing it out. I am not a company but if your argument is that you think the equation isn't up for the job, I agree, but for a completely different reason: it is the wrong equation! Seriously, let's get all the sums together and have a look at them.
John, the headline is "Balance sheet report "Show Exchange rate" broken when foreign currency is sold completely" I think we may have mixed our bugs.
(In reply to John Ralls from comment #16) > Sorry, I've lost track of the "round trip file". Where was that? You said "For the trial balance report we're also concerned that another equation balances:" above. the round trip is about the trial balance, I presumed you wanted to talk about the trial balance since you mentioned it.
(In reply to Wm from comment #18) > John, the headline is > "Balance sheet report "Show Exchange rate" broken when foreign currency is > sold completely" > I think we may have mixed our bugs. Yes, we've wandered off topic for this particular bug. This probably belongs in bug 797097. Is that also where your "round trip file" is?
(In reply to John Ralls from comment #20) > (In reply to Wm from comment #18) > > John, the headline is > > "Balance sheet report "Show Exchange rate" broken when foreign currency is > > sold completely" > > I think we may have mixed our bugs. > > Yes, we've wandered off topic for this particular bug. This probably belongs > in bug 797097. Is that also where your "round trip file" is? Yes. Let me know if you need help with the foreign stuff.
(In reply to Christopher Lam from comment #9) > I'll leave it to jralls to decide if this fix is the right answer here. I'm > still trying to figure out commodity-utilities! It's either that or --- a/gnucash/report/report-system/commodity-utilities.scm +++ b/gnucash/report/report-system/commodity-utilities.scm @@ -759,6 +759,7 @@ construct with gnc:make-gnc-monetary and gnc:monetary->string instead.") exchangelist)) (foreign-amount (gnc:gnc-monetary-amount foreign))) (if (or (not pair) + (not (cadr pair)) (zero? foreign-amount)) 0 (gnc-numeric-mul foreign-amount But yours saves an extra compare so it's marginally faster. I don't think either is more expressive of intent so let's go with yours.
fixed in maint for 3.5. further price related discussion must take place in bug 797097 or bug 797138
