Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

close command shows wrong transaction price for multi-priced balances #1035

Closed
lestephane opened this issue Jun 4, 2019 · 10 comments
Closed

close command shows wrong transaction price for multi-priced balances #1035

lestephane opened this issue Jun 4, 2019 · 10 comments
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. close

Comments

@lestephane
Copy link

I'm unable to explain the output I'm getting from hledger close

hledger prices correctly calculates the prices for a specific commodity whose purchase was done using total cost (ie @@) postings. But hledger close does not produce a total cost in line with that price.

I have journal entries where BTCs are purchased using total price (ie @@) postings

finance $ grep -r -A2 "12/31 To BTC Savings" 
2018-EUR.journal:2018/12/31 To BTC Savings    ; category:general
2018-EUR.journal-    Personal:Assets:Girokonto:Revolut:EUR                   -0,400000000 EUR = EUR78,71
2018-EUR.journal-    Personal:Assets:Savings:Revolut:BTC      0,00011812 BTC @@ EUR0,40000000
--
2018-EUR.journal:2018/12/31 To BTC Savings    ; category:general
2018-EUR.journal-    Personal:Assets:Girokonto:Revolut:EUR                   -7,200000000 EUR = EUR66,31
2018-EUR.journal-    Personal:Assets:Savings:Revolut:BTC      0,00213894 BTC @@ EUR7,20000000

The hledger-derived unit prices of said BTC commodity look fine too:

finance $ hledger prices --costs BTC | grep 2018-12-31
P 2018-12-31 BTC EUR3386,38673
P 2018-12-31 BTC EUR3366,153328

I do not have any 'P' directives anywhere in my journals, and am simply relying on transaction prices for the commodity prices to be determined by hledger automatically.

It's when I run hledger close command that I get what is (to me) unexpected.

finance $ hledger close -e 2019 --closing BTC 
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,2140556 BTC @@ EUR0,10000000 = 0 BTC
    equity:closing balances

finance $ hledger-ui --version
hledger-ui 1.14.2

Expected behaviour

2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,2140556 BTC @@  EUR720,543970317 = 0 BTC
    equity:closing balances

EUR720,543970317 is 0,2140556 * 3366,153328(last price of 2018)

the manual section for hledger close does not say anything about how transaction prices are determined in hledger close postings, so my expectation may well be off the mark.

If this issue is a known greyzone due to hledger not supporting proper priced lot tracking (#1022), feel free to close this one, and I'll revisit when #1022 is resolved.

Current workaround

I pipe the output of the hledger close command to sed, in order to clear closing transaction prices on problematic priced lots.

$ hledger close -e 2019 BTC | sed -e "s#BTC @@ EUR.* = \(.*\) BTC#BTC = \1 BTC#g"
2018/12/31 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,02269997 BTC = 0 BTC
    equity:closing balances

2019/01/01 opening balances
    Personal:Assets:Savings:Revolut:BTC     0,02269997 BTC = 0,02269997 BTC
    equity:opening balances
@simonmichael simonmichael added A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. close labels Jun 4, 2019
@simonmichael
Copy link
Owner

Great bug report and workaround - thanks. As you point out, the close command's handling of transaction prices is sloppy. If the sum contained multiple different transaction prices, it shows only the first and discards the rest.

What's the best behaviour I wonder ?

  1. combine differently priced amounts, and don't show any transaction price in the closing transaction.
  2. keep differently priced amounts separate, as if they were separate commodities.

2 is what we do by default everywhere else, so I guess that's the answer.

@simonmichael
Copy link
Owner

This gives output like:

ghci> :main  -f e.j close BTC --closing
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0 BTC
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

ghci> :main  -f e.j close BTC --closing -B
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC           -7,6 EUR = 0 EUR
    equity:closing balances

@simonmichael
Copy link
Owner

Oh. Which has a wrong balance assertion. Maybe if I leave the transaction price in the asserted amounts it'll all work out..

@simonmichael
Copy link
Owner

Related: #824

@simonmichael
Copy link
Owner

simonmichael commented Jun 4, 2019 •
edited 

; Four possible variants of closing transaction, if we keep differently priced amounts in separate postings:

; no assertions
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000
    equity:closing balances

; assertion on last posting only
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

; assertion on each posting
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0.00213894 BTC
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC
    equity:closing balances

; The above are all valid, giving register output:
; 2018/12/31 To BTC S..  ..a:Re:BTC                    0,00011812 BTC                   0,00011812 BTC
; 2018/12/31 To BTC S..  ..a:Re:BTC                    0,00213894 BTC                   0,00225706 BTC
; 2019/06/03 closing ..  ..a:Re:BTC                   -0,00011812 BTC                   0,00213894 BTC
;                        ..a:Re:BTC                   -0,00213894 BTC                                0

; priced assertion on each posting
2019/06/03 closing balances
    Personal:Assets:Savings:Revolut:BTC    -0,00011812 BTC @@ EUR0,40000000 = 0 BTC @@ EUR0,40000000
    Personal:Assets:Savings:Revolut:BTC    -0,00213894 BTC @@ EUR7,20000000 = 0 BTC @@ EUR7,20000000
    equity:closing balances

; Not valid currently, as assertion checking ignores assertion amount prices (cf #824)
; calculated: -0.00011812, asserted:   0

@simonmichael simonmichael added help wanted needs:design To unblock: needs more thought/planning, leading to a spec/plan needs:discussion To unblock: needs more discussion/review/exploration needs:docs To unblock: needs corresponding documentation or doc updates needs:impact-analysis To unblock: needs analysis of interactions with other features, users, ecosystem labels Jun 4, 2019
@simonmichael
Copy link
Owner

simonmichael commented Jun 4, 2019 •
edited

Stepping back, the first question is do we want to preserve the "lots" formed by transaction prices (and the more sophisticated lots we hope to support in future) across close/open transactions, and eg from one year's file to the next ? I think the answer must be yes.

(Note for #1022: this demonstrates how posting date and a lot's "acquisition date" can be different. Eg I acquire a lot on 2018-06-01, and start a new file in 2019, that lot appears in the 2019 journal via a 2019-01-01 opening balances posting.)

@lestephane
Copy link
Author

I agree that preserving the lots across close/open transactions is what makes the most sense.
There's no telling when I'll sell the earlier lots of BTCs, how many years later, and when that happens it will be crucial to know how much the oldest lot cost, to sell in FIFO order (if my tax jurisdiction so requires).

It seems to me that whatever format one comes up with in #1022 for lot acquisition/deposit is what will eventually be used in hledger close --opening lot positions, and the present issue is just it's mirror inverse, that is, the format for lot disposal (hledger close --closing). Solve one and the other is solved as well. Maybe I should close this issue, and follow or join the discussion in #1022?

@simonmichael
Copy link
Owner

simonmichael commented Jun 5, 2019 •
edited

Let's leave this one open, the close command really is showing bogus information here. An interim fix might be needed. Eg: when an account has a multi-price balance, show one posting per price and don't add balance assertions.

@simonmichael simonmichael changed the title 'hledger close' posting price does not agree with 'hledger prices' on same date close command shows wrong transaction price for multi-priced balances Jul 8, 2019
@simonmichael
Copy link
Owner

simonmichael commented Jul 9, 2019 •
edited

Here's an example of more correctly closing a multi-priced balance. assets balance consists of 1A priced at 1B, and another 1A priced at 2B. Note:

  • each of these amounts gets closed separately
  • a balance assertion appears only on the last closing posting for each commodity (A, here)
  • the equity posting now shows the balancing amount explicitly
  • which in multicommodity cases like this, means multiple equity postings must be shown
  • the differently priced sub-balances are preserved by the close/open transactions
2019/01/01
    (assets)                        1A @ 1B

2019/01/02
    (assets)                        1A @ 2B

$ hledger close assets -p 2019
2019/12/31 closing balances
    assets                         -1A @ 1B
    assets                         -1A @ 2B = 0A
    equity:closing balances         1A @ 1B
    equity:closing balances         1A @ 2B

2020/01/01 opening balances
    assets                          1A @ 1B
    assets                          1A @ 2B = 2A
    equity:opening balances        -1A @ 1B
    equity:opening balances        -1A @ 2B

Until now, we have left the equity posting's balance blank, so that only one equity posting was needed. I tried adding the -x/--explicit flag to make this optional. But I think it's best to just always show it explicitly. If someone prefers the implicit form it's an easy manual edit.

Here's a more complex case that I found in the tests, updated for the new behaviour:

# 8. Closing a multi-priced balance, a more complex example.
<
2016/01/31
    liabilities:employer                      $5,000.00
    income:salary

2016/02/05
    liabilities:employer                     $-5,000.00 @ 0.95 EUR
    expenses:tax                               1,852.50 EUR
    assets:bank                                2,897.00 EUR
    liabilities:employer

2016/02/29
    liabilities:employer                      $5,000.00
    income:salary

2016/03/04
    liabilities:employer                     $-5,000.00 @ 0.93 EUR
    expenses:tax                               1,813.50 EUR
    assets:bank                                2,836.00 EUR
    liabilities:employer

; Without these declarations, the closing/opening entries below will
; cause decimal marks to be misparsed. (How ?)
;commodity $1,000.00
;commodity 1,000.00 EUR

$ hledger -f- close -p 2016 assets liabilities
2016/12/31 closing balances
    assets:bank                       -5,733 EUR = 0 EUR
    liabilities:employer                $-10,000
    liabilities:employer       $5,000 @ 0.93 EUR
    liabilities:employer       $5,000 @ 0.95 EUR = $0
    liabilities:employer                  -1 EUR = 0 EUR
    equity:closing balances           $10,000.00
    equity:closing balances    $-5,000.00 @ 0.93 EUR
    equity:closing balances    $-5,000.00 @ 0.95 EUR
    equity:closing balances         5,734.00 EUR

2017/01/01 opening balances
    assets:bank                         5,733 EUR = 5,733 EUR
    liabilities:employer                  $10,000
    liabilities:employer       $-5,000 @ 0.93 EUR
    liabilities:employer       $-5,000 @ 0.95 EUR = $0
    liabilities:employer                    1 EUR = 1 EUR
    equity:opening balances           $-10,000.00
    equity:opening balances    $5,000.00 @ 0.93 EUR
    equity:opening balances    $5,000.00 @ 0.95 EUR
    equity:opening balances         -5,734.00 EUR

>=0

Let me know what you think.

simonmichael added a commit that referenced this issue Jul 9, 2019
@simonmichael
close: preserve transaction prices (costs) accurately (#1035)
6c2398e 
Transaction prices were being collapsed/misreported after close/open;
this is fixed. Now each separately-priced amount gets its own posting,
and only the last of these (for each commodity) gets a balance
assertion. Also the equity posting's amount is now always shown
explicitly, which in multicommodity situations means that multiple
equity postings are shown. The upshot is that a balance -B report
will be unchanged after closing & opening transactions.
@simonmichael simonmichael removed help wanted needs:design To unblock: needs more thought/planning, leading to a spec/plan needs:discussion To unblock: needs more discussion/review/exploration needs:docs To unblock: needs corresponding documentation or doc updates needs:impact-analysis To unblock: needs analysis of interactions with other features, users, ecosystem labels Jul 9, 2019
@lestephane
Copy link
Author

Looks fine to me, thanks!

@simonmichael simonmichael mentioned this issue Jan 4, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. close
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@simonmichael @lestephane