GnuCash
Contact   Instructions
Bug 797786 - New Balance Sheet does not balance when multiple currencies and commodities (stock holdings) are included
Summary: New Balance Sheet does not balance when multiple currencies and commodities (...
Status: NEW
Alias: None
Product: GnuCash
Classification: Unclassified
Component: Reports (show other bugs)
Version: 3.903
Hardware: PC Windows
: Normal normal
Target Milestone: ---
Assignee: reports
QA Contact: reports
URL:
Whiteboard:
Keywords:
Depends on: 797796 797813
Blocks:
  Show dependency tree
 
Reported: 2020-06-06 16:11 EDT by CDB-Man
Modified: 2021-06-24 17:43 EDT (History)
7 users (show)

See Also:


Attachments
2020-06-07 00.06 Capture - screenshot of new BS report (147.76 KB, image/png)
2020-06-07 00:09 EDT, CDB-Man
no flags Details
suggested patch (3.70 KB, patch)
2020-06-07 06:23 EDT, Christopher Lam
no flags Details
candidate balsheet-pnl.scm (60.61 KB, text/plain)
2020-06-07 12:13 EDT, Christopher Lam
no flags Details
trial reports (22.67 KB, application/zip)
2020-06-08 07:40 EDT, Christopher Lam
no flags Details
erm use this one instead (64.40 KB, text/plain)
2020-06-08 07:45 EDT, Christopher Lam
no flags Details

Description CDB-Man 2020-06-06 16:11:44 EDT
On the new balance sheet in nightly:
> gnucash-3.903-2020-06-06-git-3.903-17-ge4e36e684+.setup.exe

The output does not balance, and it is also different than the output of the original balance sheet report (report totals are not the same as the old report).

P.S. The old layout was better.  However, the new treatment for "parent account amounts include children" is a welcome change -- have the parent show the total, and any amounts in the parent itself being displayed as its own line within the subtotal.

[2020.06.06 15:44:28] <CDB-Man_> chris: i see you made some changes to the balance sheet? layout changes aside, my balance sheet no longer balances... usiing nearest in time price source
[2020.06.06 15:49:05] <jralls> Wow, that layout *really* sucks.
[2020.06.06 15:49:47] <CDB-Man_> i like the new treatment for "parent account amount sinlcude children -- thats a good idea
[2020.06.06 15:49:56] <CDB-Man_> but the rest of the layout changes are.... meh
[2020.06.06 15:50:04] <CDB-Man_> and more importantly, it does not balance
[2020.06.06 15:52:05] <jralls> Are you sure? The Liabilities + Equity line is gone so you have to add them together and compare that to assets.
[2020.06.06 15:52:47] <CDB-Man_> jralls: hmm, did not notice that at all (and that definitely needs to be added back) -- let me hand check the math
[2020.06.06 15:53:09] <CDB-Man_> still does not balance
181.bstnma.fios.verizon.net) has joined #gnucash
[2020.06.06 15:54:20] <jralls> Have you accounted for all of your capital gains?
[2020.06.06 15:54:41] <CDB-Man_> chris: I think if you're going to re-design fundamentally these core reports, they should be moved to a beta report -- just like you did for the accounts receivable customer balance one
[2020.06.06 15:55:03] <CDB-Man_> jralls: yeah, unrealized gains and everything are included -- i'm out by $~60
[2020.06.06 15:55:11] <CDB-Man_> it's definitely some exchange related difference
[2020.06.06 15:55:20] <CDB-Man_> all accounts selected for report
[2020.06.06 15:55:32] <jralls> He did that. They were in beta for most of 3.x and are being moved to mainstream for 4.0
[2020.06.06 15:57:59] <jralls> Hmm, didn't we talk last week about you doing some funny cross-expensing CAD and JPY? Did you clean all of that up?
[2020.06.06 15:58:18] <jralls> er, *with* CAD and JPY...
[2020.06.06 15:58:19] <CDB-Man_> that's all been cleaned up, and it was fine with the old rpeorts
[2020.06.06 15:58:24] <CDB-Man_> that is not the cause of this
[2020.06.06 15:59:26] <CDB-Man_> this is purely a difference due to reporting; old b/s i/s are fine -- I had installed the most recent nightly to look at the reconciliation report credit balance issue (https://bugs.gnucash.org/show_bug.cgi?id=797770 fixed now) then saw these changed b/s i/s reports
[2020.06.06 16:00:09] <CDB-Man_> "most recent nightly" being gnucash-3.903-2020-06-06-git-3.903-17-ge4e36e684+.setup.exe
[2020.06.06 16:00:26] <CDB-Man_> reverting back to gnucash-3.902-2020-05-23-git-3.902-157-gd8aecf969+.setup.exe for now... I need my statements to balance
[2020.06.06 16:04:40] <CDB-Man_> yep. downgrading again, the reports all balance, and the totals are different, so it's definitely a reporting issue
[2020.06.06 16:05:08] <jralls> CBD-Man_, please file a bug.
Comment 1 Christopher Lam 2020-06-06 16:35:58 EDT
https://github.com/Gnucash/gnucash/commits/maint/gnucash/report/standard-reports/balsheet-pnl.scm

This report was in beta for a while. The old report is still available hidden in --extra parameter.

For a good bug report I will need a datafile with sample data. The following will restore the Liability&Equity line -- a forgotten line:

modified   gnucash/report/reports/standard/balsheet-pnl.scm
@@ -1107,6 +1107,17 @@ also show overall period profit & loss."))
                                    retained-earnings-fn))))
          #:negate-amounts? #t)
 
+        (add-to-table multicol-table-right (_ "Liability and Equity")
+                      (append liability-accounts equity-accounts
+                              (if common-currency
+                                  (list (vector (_ "Unrealized Gains")
+                                                unrealized-gain-fn)
+                                        (vector (_ "Retained Earnings")
+                                                retained-earnings-fn))
+                                  '()))
+                      #:negate-amounts? #t #:show-title? #f
+                      #:show-accounts? #f #:show-total? #t)
+
         (if (and common-currency show-rates?)
             (add-to-table multicol-table-right (_ "Exchange Rates")
                           asset-liability
Comment 2 CDB-Man 2020-06-06 16:38:36 EDT
I'll try to create a sample file that can replicate the issue -- it's going to take some effort to replicate multiple currencies and stock holdings.
Comment 3 CDB-Man 2020-06-06 20:23:55 EDT
Well, in terms of creating an example file...:
[2020.06.06 17:23:27] <CDB-Man_> okay, i think i have a sufficiently broken example for Chris now
[2020.06.06 17:23:51] <CDB-Man_> ... celebrated too early... wrong file
[2020.06.06 17:25:28] <CDB-Man_> Drat. replicating this erro in a new file is gonig to be harder than I thought... my actual ledger is far too complex (500+ accounts) for me to easily pinpoint

Now, onto some discussion as to what I have learned so far.
1. In short, I'm fairly confident (90%+ probability) that the variance is caused by how the retained earnings accumulator is handling FX conversion of accumulated USD transactions.
2. That being said, there's no obvious trigger.

[2020.06.06 17:44:50] <CDB-Man> hmm, on initial inpsection, it might be because the new balance sheet report does not consdier closed book entries
[2020.06.06 17:45:11] <CDB-Man> retained earnings on my legacy b/s is $6k, on the new one is $113k
[2020.06.06 17:45:29] <CDB-Man> (the auto-calculated one, that is)
[2020.06.06 17:45:41] <jralls> That should be a feature, right? Closed book entries should be flushed to the actual retained earnings.
[2020.06.06 17:46:38] <CDB-Man> not sure what you mean by "feature" in this context, but I do use the "close books" tool to hard close and reduce all the P&L balances to 0
[2020.06.06 17:46:57] <CDB-Man> the legacy report reflects this correctly, as the retained earnniga accumulator only shows $6k, which is the current year P&L
[2020.06.06 17:47:13] <CDB-Man> the new report shows $113k, which is the lifetime amount covered by this gnucash file
[2020.06.06 17:47:35] <mohave> jralls, was not expecting it to and that is not what I intended to imply.
[2020.06.06 17:47:52] <CDB-Man> part of the change in behaviour is likely brought on by the fact that the new report now allows for some date range options to have balance sheets over time...
[2020.06.06 17:48:25] <jralls> Which is rubbish, there's no such thing as a balance sheet over time.
[2020.06.06 17:48:49] <CDB-Man> if you chek the general tab of the report settings
[2020.06.06 17:48:58] <CDB-Man> that's what the "period duration" stuff seems to try and do....
[2020.06.06 17:49:13] <CDB-Man> but otherwise I agree with you completely, balance sheets are at a point in time
[2020.06.06 17:50:20] <CDB-Man> i'm not quite sure what it tries to achieve, and also question the accuracy
[2020.06.06 17:50:54] <CDB-Man> the fact that the accumulator picks up on lifetime file P&L entries, probably also has something to do with my $60 variance not balancing, due to how that affects FX
[2020.06.06 17:51:02] <jralls> Maybe he has it in mind as some sort of diagnostic, creating a set of shadow accounts with only the transactions for a period. But it would be a rare period indeed where the inflows and outflows balanced perfectly.
[2020.06.06 17:52:05] <jralls> Well, comparing the old and new balance sheet numbers, what's different? IOW are total Assets the same on both reports, etc.
[2020.06.06 17:52:38] <CDB-Man> it seems that the period feature allows for creating comparative columns... if that's the case, i would expect the output per column to 100% agree to the legacy report
[2020.06.06 17:52:46] <CDB-Man> jralls: the asset side agrees 100%
[2020.06.06 17:53:14] <CDB-Man> all the variance is on equity
[2020.06.06 17:53:32] <jralls> Ah, right, it's not a balance sheet over time it's a two-column balance sheet with first date, second date.
[2020.06.06 17:54:05] <CDB-Man> I can see my manual retailed earnings account is $~100k on legacy (which is accurate, it's the total of all the closing entries in this file)
[2020.06.06 17:54:26] <CDB-Man> whereas on the new report, the balance is interstingly $19.90 (instead of $0 as I was expecting)
[2020.06.06 17:54:47] <CDB-Man> the accumulator retained earnings line, legacy report is $6k, new report is $113k, so that's where most of the balance went
[2020.06.06 17:55:04] <jralls> 6K + 100K != 113K.
[2020.06.06 17:55:10] <CDB-Man> the trading gains also mismatched, $91,276.72 legacy vs $9,217.94 new
[2020.06.06 17:57:06] <jralls> Getting closer, and supports your supposition earlier that exchange rates might be involved.
[2020.06.06 17:57:21] <CDB-Man> <jralls> 6K + 100K != 113K. <-- to be more precise: legacy $107,685.9 manual retaineed earnings + 5969.31 accumulator = 113,655.21.  new report $19.90 manual + 113,635.31 = 113,655.21
[2020.06.06 17:57:37] <CDB-Man> hmm, 
[2020.06.06 17:57:57] <CDB-Man> looks like if we set aside the "ignored" hard close entries for at bit, at least the total of retained earnings is correct
[2020.06.06 17:58:24] <CDB-Man> but for sure, it's the FX / unrealized that's an issue, and given this ignoring of hard close entries, likely the culprit
[2020.06.06 17:58:52] <CDB-Man> I'm curious why this $19.90 balance in RE is *not* being ignored like the other 99.99% of the balance....
[2020.06.06 17:59:11] <CDB-Man> $19.90 is 2 broker trade commissions at $9.95 each
[2020.06.06 18:00:02] <jralls> Oh, interesting. No doubt you've got more than 2...
[2020.06.06 18:00:37] <CDB-Man> Indeed, hundreds of equity trades
[2020.06.06 18:00:38] <jralls> Maybe there are 2 where you did something different, like rolling the commission into the trade net?
[2020.06.06 18:00:51] <CDB-Man> but for some reason, this particular one was not ignored
[2020.06.06 18:01:00] <CDB-Man> yeah, I'm trying to see if I can pinpoint
[2020.06.06 18:01:39] <CDB-Man> the $19.90 being precise dollars in CAD, it's won't be the culprit of the FX difference in the unrealized amount, and even if it was, $19.90 is not enough to cause the ~$60 FX variance
[2020.06.06 18:02:33] <jralls> So there's another imbalance besides R/E?
[2020.06.06 18:04:13] <CDB-Man> 1. The unrealized gains is clearly different between report versions
[2020.06.06 18:04:13] <CDB-Man> 2. Given that the new report "ignores" closing entries, and keeps all the P&L in the accumulator for R/E (my manual actual R/E account is nil), i would have expected it to be nil, but instead there's $19.90 that was "not ignored" and still sits there
[2020.06.06 18:04:29] <CDB-Man> #2 is not likely the cause of the difference in #1, it's just an interesting observation
[2020.06.06 18:04:57] <CDB-Man> aha, I've found the answer to the $19.90 quirk
[2020.06.06 18:05:50] <CDB-Man> okay, #2 is resolved, so you can disregard that one... it's really just why is there a difference in the unrealized gain
[2020.06.06 18:06:58] <jralls> What's the cause of the 19.90 and does fixing it make the out-of-balance ~$40 or ~$80?
[2020.06.06 18:07:34] <CDB-Man> working on it.... this $19.90, while identified, is not an easy fix...
[2020.06.06 18:12:08] <CDB-Man> the difference has changed slightly, the out of balance is now $58.79
[2020.06.06 18:13:39] <CDB-Man> ... nvm, the old out of balance was $91,276.72 legacy vs $91,217.94 = 58.78
[2020.06.06 18:13:44] <CDB-Man> so no impact
[2020.06.06 18:14:58] <CDB-Man> curisouly, if I run the new b/s dated 2018-12-31, there is only a 1 cent out of balance, but dated 2019-12-31 or dated 2020-06-06 (today) there is a non-rounding difference of this $~58
[2020.06.06 18:15:30] <CDB-Man> there were definitely ore f/x transiactions in 2019 vs 2018
[2020.06.06 18:15:46] <CDB-Man> that the legacy report handles, but not the new one, it seems
[2020.06.06 18:16:23] <CDB-Man> both using nearest in time
[2020.06.06 18:19:29] <jralls> You could find the first day of the larger imbalance by comparing 30 June 2019, if it's out 30 March if not 30 Sept and so on. That should make it possible to identify a transaction to analyze.
[2020.06.06 18:26:30] <CDB-Man> Good idea jralls , I'll try that after getting some dinner. After doing a quick analysis, I feel like the issue may relate to how the new report doesn't properly handle the 4010 yen that is sitting in my wallet
[2020.06.06 18:26:39] <CDB-Man> To be confirmed in an hour or so
[2020.06.06 18:27:44] <jralls> But that's only $36.59.
[2020.06.06 18:28:05] <CDB-Man> USD. Remember, I'm in CAD
[2020.06.06 18:29:59] <jralls> Right. But that's only $49.15.
[2020.06.06 18:30:36] <CDB-Man> The gross impact may be larger, due to the swing in buy price vs today's price too
[2020.06.06 18:32:00] <jralls> So you might have bought it when the CAD was higher and if GnuCash is ignoring it then it would be out by that much more. OK.
[2020.06.06 18:32:21] <jralls> Simple test, just delete it and see if the two reports agree. But after dinner.
[2020.06.06 19:04:20] <CDB-Man> okay, so working backwards, its definitely the JPY transactions that the new report handles dirrerently than the old
[2020.06.06 19:05:04] <CDB-Man> cant say affirmatively yet though... i have JPY transactions from 2019-10-15 through to 2019-11-30, but as at 2019-10-31 it still balances
[2020.06.06 19:05:12] <CDB-Man> only in Nov does it start to not balance
[2020.06.06 19:07:38] <CDB-Man> hmm, 2019-10-30 balances, 2019-10-31 does not... lets see what's booked on the 31st... out of balance $22.77
[2020.06.06 19:17:42] <CDB-Man> ... there's nothing obviously out of place with transactions on Oct 31st...
[2020.06.06 19:18:29] <jralls> Are there any JPY transactions at all that day?
[2020.06.06 19:19:36] <CDB-Man> yes, but the exact same transactions also happened on Oct 30th
[2020.06.06 19:19:43] <CDB-Man> ATM withdrawal
[2020.06.06 19:20:51] <CDB-Man> payrolll deposit hits on 31st, but that's all in CAD and same as prior pay period
[2020.06.06 19:21:03] <CDB-Man> I received a USD dividend, but that's the same transaction every quarter
[2020.06.06 19:21:26] <CDB-Man> USD 13.84, so not enough for the $22.77 diff
[2020.06.06 19:23:37] <CDB-Man> this is very odd that the new report generates a diff, but the old report does not
[2020.06.06 19:23:47] <CDB-Man> let's see what happens if i run the report not in common currency
[2020.06.06 19:24:15] <CDB-Man> oh wait, the legacy report only has common currency as a option...
[2020.06.06 19:25:18] <CDB-Man> interesting, running in non common currency, the new report breaks out the R/E accumulator into JPY, CAD, adn USD
[2020.06.06 19:25:45] <CDB-Man> JPY is $0, which makes sense as I do not have any JPY denominated gnucash P&L accounts
[2020.06.06 19:30:38] <CDB-Man> if in native currency, I cna tie the R/E accumulator to transactions, then I can at least eliminate accumulator issues as a cause
[2020.06.06 19:30:48] <CDB-Man> and then we can zero in on FX
[2020.06.06 20:20:11] <CDB-Man> jralls: well, on an individual basis, when using the new b/s report and disabling common currency, and reconciling the changes between the report on 2019-10-30 and 2019-10-31 on a per currency basis, the change in accumulator for CAD and USd both check out to the underlying transactions
[2020.06.06 20:20:22] <CDB-Man> there's no JPY denominated transactions on this date
[2020.06.06 20:20:37] <CDB-Man> so the delta in the reports is purely based on how the USD is being converted to CAD, it seems
Comment 4 CDB-Man 2020-06-06 20:54:37 EDT
Some further commenting... it seems the issue is not the retained earnings accumulator.  Instead, for a yet to be discovered reason, the unrealized gains / trading gains calculation differs between the reports on 2019-10-31, despite it being the same on 2019-10-30 and no new transaction types introduced on 2019-10-31.


[2020.06.06 20:26:48] <CDB-Man> when I enable "show exchange rates, it only shows the rate to 2 decimal places.  ie US$1.00 = $1.3100 ... even though in price database it stores it to 4.  can I get the report to show more somehow, so that I can see if it is a rate variance?
[2020.06.06 20:27:09] <jralls> CDB-Man Interesting. There's an exchange rates option that will print the rates at the bottom of the report. Does that shed any light on it?
[2020.06.06 20:28:27] <CDB-Man> both reports are on "nearest in time (to the report date, 2019-10-31)" -- when I change to "most recent (i.e. use the fx rate as of today, 2020-06-06)", a difference still persists
[2020.06.06 20:29:13] <CDB-Man> so it's something to do with how the accumulator is accumulating foreign currency (USD in this case) and converting to CAD, and no obvious trigger as to why Oct 31st is the turning point
[2020.06.06 20:29:39] <CDB-Man> it's not JPY since there's no JPY P&L accounts impacted
[2020.06.06 20:32:14] <jralls> Roger on JPY. No, I don't know of any way to get it to display the rates in more than pennies. Are the USD transactions on 31 October so large that the $22 difference is in fractions of a penny?
[2020.06.06 20:33:45] <CDB-Man> the only USD transaction is $13.84 USD, and the USD accumulator change is $13.84 from Oct 30th to 31st, so that's been accumulatoed properly
[2020.06.06 20:34:12] <CDB-Man> the transaction splits are the same as the dividend paid the quarter prior, so it's not like the split is funky or anything
[2020.06.06 20:35:04] <CDB-Man> the new b/s is _lower_ than the legacy b/s by $22.57 in CAD terms
[2020.06.06 20:35:36] <CDB-Man> when I reverse engineer the FX rate, it is 1.3126 which is the last fx rate in my price database, so that checks out
[2020.06.06 20:35:57] <CDB-Man> it's too bad the legacy report doesn't have an option no not report in common currency, else i could do some more isolation there
[2020.06.06 20:36:30] <CDB-Man> i was about to say its possible the legacy report is actualy the one that's incorrect.... but that wouldn't make sense, as th elegacy report balances
[2020.06.06 20:36:34] <CDB-Man> it's the new one that doe not
[2020.06.06 20:37:02] <jralls> I dunno. I guess we'll have to wait for chris to notice the bug. He can probably supply some scheme for you to paste in and extract more information from the reprots.
[2020.06.06 20:37:08] <CDB-Man> the asset side of the report is the same for both, so they are using the same logic for FX on that side
[2020.06.06 20:37:14] <CDB-Man> and there's no deviation on Oct 30th or 31sr
[2020.06.06 20:38:21] <CDB-Man> hmm, the manual retained earnings on legacy + the accumulator retained earnings on legacy = the retained accumulator on the new report....? interesting, I think i need to prod around a bit more
[2020.06.06 20:38:28] <CDB-Man> and double check that math
[2020.06.06 20:39:30] <CDB-Man> okay, so the issue is NOT in the r/e accumulator
[2020.06.06 20:39:35] <CDB-Man> it's actually in the unrealized gains
[2020.06.06 20:40:05] <CDB-Man> when I manual add what I expect to be in the accumulator per legacy report vs new report, they are the same
[2020.06.06 20:40:21] <CDB-Man> the variance is in the trading gains (legacy) vs unrealized gains (new) line item
[2020.06.06 20:40:42] <CDB-Man> this, given that it is based on the trading accounts, may be impacted by JPY cash balances
[2020.06.06 20:40:56] <CDB-Man> though again, there were JPY cash transactions that pre-date oct 31st, and they are all fine
[2020.06.06 20:41:15] <CDB-Man> e.g., I withdrew JPY from an ATm on oct 30th and 29th, and those caused no issues
[2020.06.06 20:41:36] <CDB-Man> the transaction template is the same, same 5 splits into the same accounts, so i'm not changing the process or anything
Comment 5 CDB-Man 2020-06-06 21:44:01 EDT
I think this is about as far as I can get on my own.  I tried to create a scenario in a test file, but can't replicate the issue, likely due to the complexity of my register (over 500 accounts...) and the difficulty in isolating specifically what is the cause.  I'm pretty confident it's to do with unrealized gains.

As an aside, I don't think the retained earnings accumulator should "ignore" Closing Entries as it currently does.  I want the retained earnings accumulator to equal exactly to the current year income statement; in order words, if I have hard closed prior year P&L into a retained earnings account, respect that and don't put it into the accumulator (this is how the legacy report works).

====
[2020.06.06 21:30:47] <CDB-Man> hmm, if i run the legacy report with ONLY my CURRENCY trading accounts selected, it reports trading loss of $126k, whereas the new report shows $0.00. i was hoping to check all the trading account with both reports, to see if they were accumulating trading gains differently
[2020.06.06 21:30:51] <CDB-Man> looks like that wont be possible
[2020.06.06 21:33:18] <CDB-Man> on the new report, if I select only lets say my JPY cash asset, then on the right hand side it will show the associated currency trading gain/loss amount
[2020.06.06 21:33:26] <CDB-Man> on the old report, can't get any similar functionality
[2020.06.06 21:33:59] <CDB-Man> old report it's all or nothing, select the entire currency account or dont. new report it attempts to associate only that portion of tradin gain/loss with the associated assets selected
[2020.06.06 21:38:01] <CDB-Man> the JPY balance on that day was 170,675 -- both reports outbut the same JPY balance and the same CAD balance on the asset side
[2020.06.06 21:38:30] <CDB-Man> it's only on the unrealized gains where there is a difference, but i cannot isolate the currency only amount, due to all my other stock holdings also being mixed in
Comment 6 Christopher Lam 2020-06-06 22:43:44 EDT
try:

modified   gnucash/report/reports/standard/balsheet-pnl.scm
@@ -879,6 +879,7 @@ also show overall period profit & loss."))
           (assoc-ref split-up-accounts ACCT-TYPE-EQUITY))
          (trading-accounts
           (assoc-ref split-up-accounts ACCT-TYPE-TRADING))
+         (use-trading-accts? (qof-book-use-trading-accounts (gnc-get-current-book)))
 
          (asset-liability (append-reverse asset-accounts liability-accounts))
          (income-expense (append-reverse income-accounts expense-accounts))
@@ -1097,6 +1098,9 @@ also show overall period profit & loss."))
         (add-to-table
          multicol-table-right (_ "Equity")
          (append equity-accounts
+                 (if use-trading-accounts?
+                     (list trading-accounts)
+                     '())
                  (if common-currency
                      (list (vector (_ "Unrealized Gains")
                                    unrealized-gain-fn))
Comment 7 CDB-Man 2020-06-06 23:11:30 EDT
Re the suggested fix in comment #6:


An error occurred while running the report.

          14 (apply-smob/1 #<catch-closure 1197fe50>)
In c-interface.scm:
     24:4 13 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9 12 (catch _ _ #<procedure 1501f120 at c-interface.scm:30:…> …)
In c-interface.scm:
    29:37 11 (_)
In unknown file:
          10 (eval-string "(gnc:report-run 13)" #<undefined>)
In ice-9/boot-9.scm:
   2312:4  9 (save-module-excursion _)
In ice-9/eval-string.scm:
     38:6  8 (read-and-eval #<input: string 15518f50> #:lang _)
In report-core.scm:
    737:4  7 (gnc:report-run _)
In c-interface.scm:
     66:2  6 (gnc:backtrace-if-exception _ . _)
     24:4  5 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9  4 (catch _ _ #<procedure 15025ff0 at c-interface.scm:30:…> …)
In c-interface.scm:
    28:40  3 (_)
In report-core.scm:
   739:29  2 (_)
   721:25  1 (gnc:report-render-html #<<report> type: c4173ac99b2b4…> …)
In gnucash/reports/standard/balsheet-pnl.scm:
  1099:30  0 (multicol-report-renderer _ _)

In procedure module-lookup: Unbound variable: use-trading-accounts?




=====
So, Chris has revealed that the new report does not directly consider the trading accounts when building the unrealized gain.  In theory it should not matter, but perhaps that's the first place to look.

If the new report can print out, for debug purposes, all components of the unrealized gain, then this output can be compared to eguile to help diagnose.  Since eguile 100% agrees to the legacy BS report, eguile can be used as a proxy to help us diagnose what's up with the new logic from Chris.


====
[2020.06.06 22:15:32] <chris> new-balance-sheet can compare balance sheet at separate dates.... useful for people who move money around
[2020.06.06 22:17:52] <chris> new balance-sheet also has capability to link the account split on the balance sheet date. ditto new multicolumn income-statement will link to a Transaction Report that illustrates IS period.
[2020.06.06 22:17:58] <chris> IS=income-statement
[2020.06.06 22:18:16] <CDB-Man> hmm, after printing all the fx and stock prices on both reports, they are both pulling the same pricing (notwithstanding the decimals beyond 2 that I can't see for USD and JPY...) so it's something else in the logic for unrealized gains that is different
[2020.06.06 22:18:18] <chris> CDB-Man: they were in beta for a long time, in the Experimental menu.
[2020.06.06 22:18:40] <CDB-Man> [2020.06.06 22:18:18] <chris> CDB-Man: they were in beta for a long time, in the Experimental menu. <-- ah, so that's why I didn't notice
[2020.06.06 22:18:55] <chris> CDB-Man: is the Balance Sheet eguile version correct for you?
[2020.06.06 22:19:10] <CDB-Man> hmm, i've never used eguile before -- what's the difference?
[2020.06.06 22:19:47] <chris> it's another report written by someone else, less old
[2020.06.06 22:20:28] <chris> it was meant to be the 'new-wave' of reports but they gave up halfway through
[2020.06.06 22:21:03] <CDB-Man> we'll find out momentarily
[2020.06.06 22:22:13] <CDB-Man> An error occurred while running the report.
[2020.06.06 22:22:15] <CDB-Man> well now!
[2020.06.06 22:22:28] <chris> how odd
[2020.06.06 22:22:52] <CDB-Man> [...]
[2020.06.06 22:22:52] <CDB-Man> In unknown file:
[2020.06.06 22:22:52] <CDB-Man>            0 (make-list -1 "<td class=\"empty\"></td>")
[2020.06.06 22:22:52] <CDB-Man> Value out of range 0 to 4294967295: -1
[2020.06.06 22:23:32] <CDB-Man> okay, it crashes when i limit sub accounts level to 2
[2020.06.06 22:23:39] <chris> ok
[2020.06.06 22:23:45] <CDB-Man> 3 or more is fine
[2020.06.06 22:23:47] <CDB-Man> how odd
[2020.06.06 22:23:53] <chris> mind a few observations about new balance-sheet: 1. I'm no accountant (I'm actually an MD) 2. I wrote these newer reports to scratch my own itches, and made them as good as I could 3. older reports are obviously more well tested but are a maintenance nigthmare because no one knows how they work
[2020.06.06 22:24:16] <CDB-Man> secret sauce indeed
[2020.06.06 22:24:16] <chris> 4. the new reports are swapped because generally they are nicer than old ones
[2020.06.06 22:24:21] <CDB-Man> you're doing pretty good for an MD
[2020.06.06 22:24:51] <chris> I don't mind at all whether they are swapped (I resisted) but it means subtle bugs are never found until a capable CPA takes notice
[2020.06.06 22:25:32] <CDB-Man> well, if it's any consolation, the eguile versino also does not balance
[2020.06.06 22:25:42] <chris> \o/
[2020.06.06 22:25:48] <CDB-Man> it's out by more
[2020.06.06 22:25:52] <CDB-Man> wait
[2020.06.06 22:26:02] <CDB-Man> hold on, it's presented differently, i need to scroll down more
[2020.06.06 22:26:32] <CDB-Man> it balances
[2020.06.06 22:26:42] <chris> yup. someone complained the RE section shouldn't be expanded in eguile version
[2020.06.06 22:26:48] <CDB-Man> agrees to the cent with the legacy report
[2020.06.06 22:27:17] <CDB-Man> expanding the RE secton adds no value... it's what the income statement or a statement of chagnes in equity is for
[2020.06.06 22:27:53] <CDB-Man> honestly without sending you my whole file, i dont think i can feasibly show you a dataset with my out of balance
[2020.06.06 22:28:07] <CDB-Man> i tried creating example, but nothing can match my 500 account production dataset
[2020.06.06 22:29:08] <CDB-Man> whatever eguile does for trading gains, it agrees with legacy
[2020.06.06 22:29:28] <CDB-Man> hmm
[2020.06.06 22:29:37] <CDB-Man> actually, sicne eguoille breaks down traiding gains per line item
[2020.06.06 22:29:47] <chris> ok. If you have isolated the discrepancy with Retained earnings calculator then it's a good clue
[2020.06.06 22:29:51] <CDB-Man> i might be able to use this to identify vsi a vis new report, which item is causing the out of balance
[2020.06.06 22:30:08] <CDB-Man> chris did you read the entire volume of comments in this channel / cc'd to the ticket?
[2020.06.06 22:30:24] <CDB-Man> or at least, the summary points the precede each transcript?
[2020.06.06 22:30:29] <chris> ^ loosely, not the actual numbers discussed
[2020.06.06 22:30:45] <CDB-Man> it's not the RE accumulator, it's the trading gains accumulator that is at issue
[2020.06.06 22:30:59] <CDB-Man> basically all the FX flux and the stock price flux
[2020.06.06 22:31:03] <chris> the "unrealized gains"?
[2020.06.06 22:31:23] <CDB-Man> yes
[2020.06.06 22:32:19] <CDB-Man> on oct 30th the reports ar ethe same (out by $0.01). on oct 31st, they are out by $22.57 out of balance on the new report... and the difference persists..  already analyzed all transactions on oct 31st, and it''s not a transactin issue, hence why I'm concluding its not the RE accumulator
[2020.06.06 22:32:53] <CDB-Man> (that and, I really dont like how the RE accumulator ignores closing entries as mentioned in comment #5... -- but that doesnt impact the problem at hand)
[2020.06.06 22:32:53] <chris> FWIW the old balance-sheet has a subtle bug which I didn't fix: it calculates TRADING-GAINS (i.e. from Trading-XXX splits) but chooses to show it depending on the book "Use trading gains" status... This means if user enables trading accounts, does check&repair, and disables trading accounts, the old balsheet will be incorrect
[2020.06.06 22:33:36] <CDB-Man> well, ive had them on since day 1, and the reports are immor images until oct 31st
[2020.06.06 22:33:51] <CDB-Man> if it was something that structurel, i would have expected the reports to be permanently different
[2020.06.06 22:34:00] <CDB-Man> rather than only have a difference creep up on a specific date
[2020.06.06 22:34:12] <CDB-Man> its not like i dont have stocks and foreign currency prior to that date
[2020.06.06 22:37:21] <chris> I'll follwup on bug when I can think of suitable fix candidates
[2020.06.06 22:37:34] <CDB-Man> hmm, there's no easy way to compare the eguile breakdown of trading gains vs your new version
[2020.06.06 22:37:39] <CDB-Man> since they accumulate differently
[2020.06.06 22:37:44] <chris> do you use "Trading accounts"?
[2020.06.06 22:38:01] <CDB-Man> but at least eguile matchnig legacy 100%, means that what it presents should be accurate
[2020.06.06 22:38:09] <CDB-Man> [2020.06.06 22:33:36] <CDB-Man> well, ive had them on since day 1, and the reports are immor images until oct 31st
[2020.06.06 22:38:16] <CDB-Man> so yes
[2020.06.06 22:38:19] <chris> ah
[2020.06.06 22:38:29] <chris> bingo
[2020.06.06 22:38:46] <chris> new report doesnt take them at all -- because from my experiments they didnt affect the UG at all
[2020.06.06 22:39:08] <CDB-Man> .... well...
[2020.06.06 22:39:22] <CDB-Man> again, i want to emphasize that if i date both reports 2019-10-30, they are the same
[2020.06.06 22:39:34] <CDB-Man> but when i advance the date to 2019-10-31, they mismatch, permanently
[2020.06.06 22:39:48] <CDB-Man> and its not like i didnt have forex and stocks prior to 2019-10-31
[2020.06.06 22:41:04] <CDB-Man> [2020.06.06 22:38:46] <chris> new report doesnt take them at all -- because from my experiments they didnt affect the UG at all  <-- i like how when in your new report, if i for example select only the JPY cahs balance, the equity side accumulates the unrealized gain/loss on JPY to CAD
[2020.06.06 22:41:20] <CDB-Man> in theory, the inclusing of trading accounts ought to give the exact same answer
[2020.06.06 22:41:34] <CDB-Man> the fact that it doesnt... baffle me
[2020.06.06 22:41:54] <chris> "i like how when in your new report,..." --> this 'benefit' is unintentional
[2020.06.06 22:41:58] <CDB-Man> and it baffles further that it works for all dates (that i sampled) prior to 2019-10-31, but not all dates 2019-10-31 or after
[2020.06.06 22:42:46] <CDB-Man> well, using theis benefit, i could for example pick my MSFT stock and compare it to eguile's trading gains accumulation
[2020.06.06 22:42:54] <CDB-Man> i can do this easily for stocks... not so much for currency
[2020.06.06 22:43:07] <CDB-Man> since currency is used to purchase stocks, so there's multiple layers of complexity
[2020.06.06 22:45:42] <CDB-Man> hmm, for your new report,choosing only my back accounts shows the unrealized gain on the right side, but choosing a stock account yields zeros
[2020.06.06 22:45:48] <CDB-Man> so i cant use that to diagnose
[2020.06.06 22:45:51] <chris> I gtg now, breakfast. If this upgraded report isn't accurate, maybe it should return to beta status. My time on hacking will be severely reduced in a couple of months
[2020.06.06 22:46:07] <CDB-Man> that or, i chose an empty account... i have multiple MSFT accounts...
[2020.06.06 22:46:52] <CDB-Man> well chris, the likelihood of inaccuracy is definitely proportional to the complexitty of the register... and mine is definitely high on the coplexity scale
[2020.06.06 22:47:05] <CDB-Man> i havent kicked all the tires yet on IS, but that one seems more sound
[2020.06.06 22:47:14] <chris> so, maybe jralls will prefer swap the reports back to where they were (balance-sheet, income-statement, new-aging etc)
[2020.06.06 22:47:43] <CDB-Man> aging as in A/R aging?
[2020.06.06 22:48:01] <chris> mutlicol-IS will be a hoot... I made assumptions on that one (eg what date to use for currency conversions? let's pick the end date for each period)
[2020.06.06 22:48:09] <chris> yes new-aging IMHO better than old-aging
[2020.06.06 22:48:20] <CDB-Man> i don't use that report, so can't comment on that one
[2020.06.06 22:48:24] <CDB-Man> i only use the customer report
[2020.06.06 22:48:29] <chris> that too
[2020.06.06 22:48:52] <chris> try the Customer Report Display/Links option
[2020.06.06 22:49:02] <CDB-Man> the custome one ive been using beta for a while, so tired have been kicked on that
[2020.06.06 22:49:33] <CDB-Man> ... i just noticed all my previously open customer reports are dead
[2020.06.06 22:49:38] <CDB-Man> presumably due to the switch
[2020.06.06 22:50:35] <CDB-Man> yeah, I quite like the display detailed links optiom
[2020.06.06 22:52:03] <CDB-Man> in terms of styling, i really think that in the new reports, all parent accounts need to be highlighred. either highlight yellow (on technicolour), or put in the accounting style rule on the indent to show that the indended items are a subtotal
[2020.06.06 22:52:32] <CDB-Man> <chris> I gtg now, breakfast. <-- hmm, Australia / HK timezone?
[2020.06.06 22:54:26] <CDB-Man> hmm.... yeah if i filter for my MSFT holding, your report on the equity side shows the net gain, which I can only get from eguille if I include both the MSFT tading account and the corresponding offset amount in CURRENCY:USD, and there's no easy way to pick that one out
[2020.06.06 22:54:33] <CDB-Man> from eguile
[2020.06.06 22:57:17] <CDB-Man> for debug purposes, if your new b/s report were to print the complete breakdown of the unrealized gains, the i could compare it side by side to eguile, and see where it falls apart
[2020.06.06 22:58:19] <CDB-Man> the fact that your assets agree to legacy and to eguile means it's definitely on the unrealzied size, but it still puzzles me greatly why i only see errors after 2019-10-31, especialyl after having done a detailed transacton analysis....
Comment 8 Christopher Lam 2020-06-06 23:20:20 EDT
typo instead of use-trading-accounts? it was meant to be use-trading-accts?
Comment 9 CDB-Man 2020-06-06 23:25:34 EDT
Typo is now fixed, and a different error is found:


An error occurred while running the report.

          16 (apply-smob/1 #<catch-closure 1121fe50>)
In c-interface.scm:
     24:4 15 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9 14 (catch _ _ #<procedure 146a4550 at c-interface.scm:30:…> …)
In c-interface.scm:
    29:37 13 (_)
In unknown file:
          12 (eval-string "(gnc:report-run 13)" #<undefined>)
In ice-9/boot-9.scm:
   2312:4 11 (save-module-excursion _)
In ice-9/eval-string.scm:
     38:6 10 (read-and-eval #<input: string 147a78f8> #:lang _)
In report-core.scm:
    737:4  9 (gnc:report-run _)
In c-interface.scm:
     66:2  8 (gnc:backtrace-if-exception _ . _)
     24:4  7 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9  6 (catch _ _ #<procedure 146a4420 at c-interface.scm:30:…> …)
In c-interface.scm:
    28:40  5 (_)
In report-core.scm:
   739:29  4 (_)
   721:25  3 (gnc:report-render-html #<<report> type: c4173ac99b2b4…> …)
In gnucash/reports/standard/balsheet-pnl.scm:
   1098:8  2 (multicol-report-renderer _ _)
   474:16  1 (add-multicolumn-acct-table #<<html-table> col-headers…> …)
In unknown file:
           0 (gnc-account-get-current-depth (#<swig-pointer Accou…> …))

In procedure gnc-account-get-current-depth: Wrong type argument in position 1: (#<swig-pointer Account * 13bcf4d8> #<swig-pointer Account * 13bcf5f0> #<swig-pointer Account ......... etc etc
Comment 10 Christopher Lam 2020-06-06 23:26:56 EDT
shucks.

+                 (if use-trading-accounts?
+                     (list (vector (_ "Trading Gains")
+                                   trading-accounts))
+                     '())
                  (if common-currency
                      (list (vector (_ "Unrealized Gains")
                                    unrealized-gain-fn))
Comment 11 CDB-Man 2020-06-06 23:33:09 EDT
It doesn't relent :)


===

An error occurred while running the report.

In c-interface.scm:
     24:4 19 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9 18 (catch _ _ #<procedure 110cfcf0 at c-interface.scm:30:…> …)
In c-interface.scm:
    29:37 17 (_)
In unknown file:
          16 (eval-string "(gnc:report-run 13)" #<undefined>)
In ice-9/boot-9.scm:
   2312:4 15 (save-module-excursion _)
In ice-9/eval-string.scm:
     38:6 14 (read-and-eval #<input: string 14ab9930> #:lang _)
In report-core.scm:
    737:4 13 (gnc:report-run _)
In c-interface.scm:
     66:2 12 (gnc:backtrace-if-exception _ . _)
     24:4 11 (gnc:call-with-error-handling _ _)
In ice-9/boot-9.scm:
    829:9 10 (catch _ _ #<procedure 110cfa90 at c-interface.scm:30:…> …)
In c-interface.scm:
    28:40  9 (_)
In report-core.scm:
   739:29  8 (_)
   721:25  7 (gnc:report-render-html #<<report> type: c4173ac99b2b4…> …)
In gnucash/reports/standard/balsheet-pnl.scm:
   1098:8  6 (multicol-report-renderer _ _)
   628:14  5 (add-multicolumn-acct-table #<<html-table> col-headers…> …)
   537:23  4 (add-account-row 0 #("Trading Gains" (#<swig-poin…> …)) …)
In srfi/srfi-1.scm:
   592:17  3 (map1 (0))
In gnucash/reports/standard/balsheet-pnl.scm:
   540:31  2 (_ 0)
   522:29  1 (sum-accounts-at-col (#("Trading Gains" (#<swig-…> …))) …)
In unknown file:
           0 (_ 0)

Wrong type to apply: (#<swig-pointer Account * 13cd6448> #<swig-pointer Account * 13cd6560> #<swig-pointer Account * 13cd6678> --- etc
Comment 12 Christopher Lam 2020-06-06 23:42:24 EDT
Last patch:

modified   gnucash/report/reports/standard/balsheet-pnl.scm
@@ -879,6 +879,7 @@ also show overall period profit & loss."))
           (assoc-ref split-up-accounts ACCT-TYPE-EQUITY))
          (trading-accounts
           (assoc-ref split-up-accounts ACCT-TYPE-TRADING))
+         (use-trading-accts? (qof-book-use-trading-accounts (gnc-get-current-book)))
 
          (asset-liability (append-reverse asset-accounts liability-accounts))
          (income-expense (append-reverse income-accounts expense-accounts))
@@ -1096,7 +1097,7 @@ also show overall period profit & loss."))
 
         (add-to-table
          multicol-table-right (_ "Equity")
-         (append equity-accounts
+         (append equity-accounts trading-accounts
                  (if common-currency
                      (list (vector (_ "Unrealized Gains")
                                    unrealized-gain-fn))
@@ -1107,6 +1108,23 @@ also show overall period profit & loss."))
                                    retained-earnings-fn))))
          #:negate-amounts? #t)
 
+        (add-to-table multicol-table-right (_ "Liability and Equity")
+                      (append liability-accounts
+                              equity-accounts
+                              trading-accounts
+                              (if common-currency
+                                  (list (vector (_ "Unrealized Gains")
+                                                unrealized-gain-fn))
+                                  '())
+                              (if (null? income-expense)
+                                  '()
+                                  (list (vector (_ "Retained Earnings")
+                                                retained-earnings-fn))))
+                      #:negate-amounts? #t
+                      #:show-title? #f
+                      #:show-accounts? #f
+                      #:show-total? #t)
+        
         (if (and common-currency show-rates?)
             (add-to-table multicol-table-right (_ "Exchange Rates")
                           asset-liability
Comment 13 CDB-Man 2020-06-07 00:09:41 EDT
Created attachment 373714 [details]
2020-06-07 00.06 Capture - screenshot of new BS report

See attached.  The total trading gains $41,551.83 disagrees with the unrealized gains at the bottom of $41,529.06.  However, the $41,551.83 is exactly the amount I see in guile and legacy BS report.  The individual subtotals for CURRENCY, NASDAQ, etc, all agree to guile as well.
Comment 14 Christopher Lam 2020-06-07 04:19:09 EDT
Instead of trying different patches, perhaps I'll explain them in detail and you can decide which strategy is best. This is confounded by the fact that TRADING accounts exist, and "Use Trading Accounts" option also exists.

We'd expect and hope that the existence Trading Accounts (type=TRADING, autogenerated) and state of Use-Trading-Accounts option are kept in sync. (i.e. use-trading-accounts=true and trading-accounts exist). But this is not enforced by the engine. e.g. someone may enable "use trading accounts" option, run check&repair to create the accounts, and disable the option. Result = book is "slightly broken". But I disgress.

For the following patch, I'll explain after each "+" addition:

Unstaged changes (1)
modified   gnucash/report/reports/standard/balsheet-pnl.scm
@@ -879,6 +879,7 @@ also show overall period profit & loss."))
           (assoc-ref split-up-accounts ACCT-TYPE-EQUITY))
          (trading-accounts
           (assoc-ref split-up-accounts ACCT-TYPE-TRADING))
+         (use-trading-accts? (qof-book-use-trading-accounts (gnc-get-current-book)))


[i.e. the use-trading-accts? variable queries the use-trading-accounts option, and is #true if set, and #false if unset]
 
          (asset-liability (append-reverse asset-accounts liability-accounts))
          (income-expense (append-reverse income-accounts expense-accounts))
@@ -998,8 +999,11 @@ also show overall period profit & loss."))
                             (vector-ref asset-liability-balances col-idx))
                            (asset-liability-basis
                             (vector-ref asset-liability-value-balances col-idx))
-                           (unrealized (gnc:collector- asset-liability-basis
-                                                       asset-liability-balance)))
+                           (unrealized
+                            (if use-trading-accts?
+                                (gnc:collector+)
+                                (gnc:collector- asset-liability-basis
+                                                asset-liability-balance))))


[this patch is new and now mimics the old balance-sheet. Formerly the 'unrealized gains' line would collect the difference between A&L basis and the A&L balance; now it is zero if use-trading-accts option is #false.]


                   (monetaries->exchanged
                    unrealized common-currency price-source date))))
 
@@ -1096,7 +1100,7 @@ also show overall period profit & loss."))
 
         (add-to-table
          multicol-table-right (_ "Equity")
-         (append equity-accounts
+         (append equity-accounts trading-accounts

[this part will now display equity accounts, trading accts in the Equity section of the Balsheet.]


                  (if common-currency
                      (list (vector (_ "Unrealized Gains")
                                    unrealized-gain-fn))
@@ -1107,6 +1111,22 @@ also show overall period profit & loss."))
                                    retained-earnings-fn))))
          #:negate-amounts? #t)
 
+        (add-to-table multicol-table-right (_ "Liability and Equity")
+                      (append liability-accounts
+                              equity-accounts
+                              (if common-currency
+                                  (list (vector (_ "Unrealized Gains")
+                                                unrealized-gain-fn))
+                                  '())
+                              (if (null? income-expense)
+                                  '()
+                                  (list (vector (_ "Retained Earnings")
+                                                retained-earnings-fn))))
+                      #:negate-amounts? #t
+                      #:show-title? #f
+                      #:show-accounts? #f
+                      #:show-total? #t)
+

[this part will add a final L&E section to mimic old balance-sheet. It includes Liability Accounts, Equity accounts, and possibly Unrealized Gains, and possibly Retained Earnings. It is easy to add/remove a group of accounts -- e.g. to add trading accounts, add 'trading-accounts' next to 'equity-accounts'.]

         (if (and common-currency show-rates?)
             (add-to-table multicol-table-right (_ "Exchange Rates")
                           asset-liability
Comment 15 Christopher Lam 2020-06-07 06:23:27 EDT
Created attachment 373715 [details]
suggested patch

Patch.

1. fixes legacy balance-sheet to *omit* trading-accounts if trading-accounts-option is false.

2. fixes multicolumn balance-sheet to query trading-accounts-option.

(a) option=true: lists trading accounts as part of equity section, and assume their calculation for UG is accurate

(b) option=false: omits trading accounts. calculate UG manually - difference in value and cost for A&L accounts.
Comment 16 Christopher Lam 2020-06-07 06:26:00 EDT
This comment #15 fix will assume that the trading accounts are a valid UG calculator.
Comment 17 Christopher Lam 2020-06-07 12:13:59 EDT
Created attachment 373717 [details]
candidate balsheet-pnl.scm
Comment 18 Christopher Lam 2020-06-07 19:26:29 EDT
Try this small patch which removes rounding while accumulating monetary objects:

--->snip----
modified   gnucash/report/report-utilities.scm
@@ -231,9 +231,7 @@
     ;; If no pair with this commodity exists, we will create one.
     (define (add-commodity-value commodity value)
       (let ((pair (assoc commodity commoditylist))
-            (rvalue (gnc-numeric-convert
-                     value
-                     (gnc-commodity-get-fraction commodity) GNC-RND-ROUND)))
+            (rvalue value))
 	(unless pair
 	  (set! pair (list commodity (gnc:make-value-collector)))
 	  (set! commoditylist (cons pair commoditylist)))
--->snip----
Comment 19 CDB-Man 2020-06-07 21:07:49 EDT
Hmm, this does not work... the entire report is now completely goofy... as in my total assets went from $280k to $213k, and a bunch of liability accounts now suddenly have values since a bunch of entries have been excluded.  Either I messed up, or this theoretically quick fix of removing gnc-numeric-convert isn't so quick...


[2020.06.07 20:33:04] <CDB-Man> hmm, something definitely went wrong with chris' suggested fix in https://bugs.gnucash.org/show_bug.cgi?id=797786#c18 --- i went from a 1 cent rounding diff to now a completely incorrect balance sheet.... that or I missed something up on modifying those 4 lines
[2020.06.07 20:34:25] <CDB-Man> probably some missing brackets somewhere....
[2020.06.07 20:41:57] <CDB-Man> okay, i don't think its brackets, but perhaps leaving out "gnc-commodity-get-fraction commodity" altogether wasn't the greatest idea?
[2020.06.07 20:44:28] <CDB-Man> hmm, I definitely broke something
[2020.06.07 20:49:30] <CDB-Man> when in doubt, reinstall the whole thing!
Comment 20 CDB-Man 2020-06-07 21:58:34 EDT
On a side note regarding presentation, I think it's worthwhile to have a flag "show unrealized gains detail" with the default being unselected.  When unselected, only a single "unrealized gains" line is shown (similar to the legacy report).  When selected, all the details is shown (as it stands right now).  If you're someone else mine with 50 different stock tickers, the trading section can get very long.  And at account depth 3, bumping the depth to 2 would take out detail in other areas of the report.
Comment 21 Christopher Lam 2020-06-07 22:45:06 EDT
(In reply to CDB-Man from comment #20)
> On a side note regarding presentation, I think it's worthwhile to have a
> flag "show unrealized gains detail" with the default being unselected.  When
> unselected, only a single "unrealized gains" line is shown (similar to the
> legacy report).  When selected, all the details is shown (as it stands right
> now).  If you're someone else mine with 50 different stock tickers, the
> trading section can get very long.  And at account depth 3, bumping the
> depth to 2 would take out detail in other areas of the report.

Ok just to be clear:
- trading accounts will autocalculate the unrealized gains
- when trading-accounts aren't used, the balsheet will calculate unrealized gains

When trading accounts are used, they will clutter then balsheet.

You wish the clutter to be optional: It means simply modify my patch as follows, or add option to "Show Unrealized Gains instead of Trading Accounts", and will only be useful when Trading Accounts are in use:

modified   gnucash/report/reports/standard/balsheet-pnl.scm
@@ -879,6 +879,7 @@ also show overall period profit & loss."))
           (assoc-ref split-up-accounts ACCT-TYPE-EQUITY))
          (trading-accounts
           (assoc-ref split-up-accounts ACCT-TYPE-TRADING))
+         (use-trading-accts? #f)
Comment 22 CDB-Man 2020-06-07 23:03:39 EDT
> Ok just to be clear:
> - trading accounts will autocalculate the unrealized gains
> - when trading-accounts aren't used, the balsheet will calculate unrealized gains

> When trading accounts are used, they will clutter then balsheet.

> You wish the clutter to be optional [when using trading accounts]

Correct on all counts, with the 1 addendum.

However, using:
+ (use-trading-accts? #f)

Instead of:
+ (use-trading-accts? (qof-book-use-trading-accounts (gnc-get-current-book)))

Results in out of balance; the suggested fix seems to get balsheet to calculate the amount again, and therefore the amount no longer agrees to the amount calculated with the full trading account tree... it also disagrees with legacy report now...

Not sure how technically challenging it would be, but effective, what I am saying is: have a report option to, effectively, set Unrealized Gains account depth to 1 always, vs use whatever the trading account depth is for the rest of the report.
Comment 23 CDB-Man 2020-06-07 23:07:09 EDT
Hmm, thinking out loud here, overriding the boolean to #f false basically forces it to use balsheet calc, even though the register contains trading accounts.  That is _not_ what I am asking for.  What I am saying is allow trading accounts to use a separate "level of subaccounts" from the main report; either a) 1 level, or b) the same as the rest of the report.  Overriding level = 1 would effectively display 1 line of unrealized gains, mimicking the rpesentation for a book not using trading accounts... but still allowing for correct aggregation using actual trading accounts.
Comment 24 CDB-Man 2020-06-07 23:09:32 EDT
Or put another way, instead of:
> "Show Unrealized Gains instead of Trading Accounts"

A better way to say it is:
> "Force trading accounts to only display 1 level of sub-account
> (i.e. a different level setting than for the rest of the report"
Comment 25 Christopher Lam 2020-06-07 23:10:33 EDT
(In reply to CDB-Man from comment #23)
> Hmm, thinking out loud here, overriding the boolean to #f false basically
> forces it to use balsheet calc, even though the register contains trading
> accounts.  That is _not_ what I am asking for.  What I am saying is allow
> trading accounts to use a separate "level of subaccounts" from the main
> report; either a) 1 level, or b) the same as the rest of the report. 
> Overriding level = 1 would effectively display 1 line of unrealized gains,
> mimicking the rpesentation for a book not using trading accounts... but
> still allowing for correct aggregation using actual trading accounts.

I understand; it seems the UG calculator isn't giving good results. I can try hide TradingAccounts detail but I'd rather fix the UG calculator.
Comment 26 CDB-Man 2020-06-07 23:12:36 EDT
> I understand; it seems the UG calculator isn't giving good results.
In theory, there ought not to be a difference between UG and trading accounts... it really bothers me that there is a difference.  And it's even more odd that, for my gnucash book anyways, the cutoff for error is pre- 2019-10-31 vs post- 2019-10-31, with no obvious trigger.
Comment 27 Christopher Lam 2020-06-08 00:16:17 EDT
(In reply to CDB-Man from comment #26)
> > I understand; it seems the UG calculator isn't giving good results.
> In theory, there ought not to be a difference between UG and trading
> accounts... it really bothers me that there is a difference.  And it's even
> more odd that, for my gnucash book anyways, the cutoff for error is pre-
> 2019-10-31 vs post- 2019-10-31, with no obvious trigger.

I think this is a good bug. Reviewing balsheet-pnl.scm is defining (exchange-fn ...) twice and this is not ever right. Will offer fix tonight.
Comment 28 Christopher Lam 2020-06-08 07:40:38 EDT
Created attachment 373718 [details]
trial reports

trial
Comment 29 Christopher Lam 2020-06-08 07:45:39 EDT
Created attachment 373719 [details]
erm use this one instead
Comment 30 Christopher Lam 2020-06-08 07:48:51 EDT
(In reply to CDB-Man from comment #24)
> Or put another way, instead of:
> > "Show Unrealized Gains instead of Trading Accounts"
> 
> A better way to say it is:
> > "Force trading accounts to only display 1 level of sub-account
> > (i.e. a different level setting than for the rest of the report"

If we want the Equity section to display all equity accounts but only 1 level for Trading Accounts, then: No, it'll be too difficult. If you can agree for Level 1 for Equity+Trading accounts, then:

(add-to-table
         multicol-table-right (_ "Equity")
         (append equity-accounts
                 (cond
                  (use-trading-accts? trading-accounts)
                  (common-currency (list (vector (_ "Unrealized Gains")
                                                 unrealized-gain-fn)))
                  (else '()))
                 (if (null? income-expense)
                     '()
                     (list (vector (_ "Retained Earnings")
                                   retained-earnings-fn))))
         #:negate-amounts? #t
         #:depth-limit 0                 ;<----note depth limit 0 or 1
         )
Comment 31 CDB-Man 2020-06-08 23:01:43 EDT
The proposed version in comment #29 works.  Along with the patch from bug 797793 comment #2.

Reading some more relevant snippets into the record...

[2020.06.08 10:39:40] <chris> CDB-Man_: I've just pushed commits that I'm 100% sure about to master. 797786 still has some more changes to try fix FX date chosen, untested.
[2020.06.08 10:40:13] <chris> CDB-Man_: also 0.01 rounding not pushed; I'm not sure why your test produced such wide discrepancy.
[2020.06.08 10:40:38] <chris> ergo: try 797786 #c29 still!
[2020.06.08 10:41:01] <chris> (in addition to latest master)
[2020.06.08 10:44:51] <CDB-Man_> chris: cool, I'll try the next nightly tomorrow
[2020.06.08 10:45:18] <CDB-Man_> Regarding the collapsing of trading account details, is the difficulty because it's grouped under equity?
[2020.06.08 10:45:45] <CDB-Man_> If so, what if we created a 4th category, ie assets liabilities equity trading
[2020.06.08 10:46:09] <CDB-Man_> Then trading can either be 0 depth or report depth
[2020.06.08 10:51:00] <chris> too complex
[2020.06.08 10:51:13] <chris> hmm maybe
[2020.06.08 10:51:16] <chris> still ugly

[2020.06.08 10:52:08] <chris> is trading conceptually separate from equity?
[2020.06.08 10:52:30] <chris> if so does RE belong to Equity then?
[2020.06.08 10:57:06] <CDB-Man_> [10:52:08] <+chris> is trading conceptually separate from equity?
[2020.06.08 10:57:11] <CDB-Man_> "it depends"
[2020.06.08 10:57:44] <CDB-Man_> It's a component of equity, but typically a specific subcomponent
[2020.06.08 10:58:09] <CDB-Man_> For example under IFRS, unrealized gains on investments is recorded as AOCI
[2020.06.08 10:58:21] <CDB-Man_> Which stands for accumulated other comprehensive income
[2020.06.08 10:58:53] <CDB-Man_> Retained earnings however is definitely standard equity

[2020.06.08 11:34:27] <chris> it's *possible* to display Trading in its own line vs UG, but I'd seriously still like to fix the UG calculator
[2020.06.08 11:59:28] <CDB-Man_> Well yeah I agree, but until that's done, this is the easiest Band-Aid
Comment 32 Christopher Lam 2020-06-08 23:40:06 EDT
CDB-Man: Thank you for being so thorough in helping fix BS (and IS). I think it would be safer to revert the multicolumn reports balsheet-pnl.scm back to beta in the experimental submenus for now?

My next change to balsheet-pnl.scm will be to ensure BS respects closing entries for all sections, and IS ignores closing entries.
Comment 33 CDB-Man 2020-06-08 23:47:46 EDT
[2020.06.08 19:40:42] <chris> fao jralls it may be better to revert balsheet-pnl back to beta until CDB-Man is finished kicking it
[2020.06.08 20:34:50] <jralls> chris re balancesheet-pnl OK. It sounds like it perhaps needs a bit of a re-think.

1. Agreed with you and jralls re: marking it beta again for now
2. TODO: respect closing entries
3. TODO: FX issue that you discovered
4. TODO: unrealized gains / trading gains (hopefully #3 helps)
Comment 34 Christopher Lam 2020-06-09 11:12:23 EDT
1. done in master - back to experimental submenu - commit b311cc868
2. done in master - bs has closing, pnl ignores closing - 8f60a6c61 and b000d4114
3. PENDING
4. PENDING
Comment 35 CDB-Man 2020-06-13 17:05:26 EDT
> 2. done in master - bs has closing, pnl ignores closing - 8f60a6c61 and 
> b000d4114

Just tested nightly:
> gnucash-3.904-2020-06-13-git-3.904-36-g51d00fcbe+.setup.exe

And the closing entries issue is corrected.
Comment 36 CDB-Man 2020-06-21 18:11:54 EDT
Refer also to newly filed bug 797813, which is an enhancement request that likely has an impact on the resolution of this bug.

Note You need to log in before you can comment on or make changes to this bug.