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.

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

transaction modifiers do not multiply total-priced amounts correctly #928

Closed
DanMeakin opened this Issue Nov 13, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@DanMeakin

DanMeakin commented Nov 13, 2018

I have encountered an issue since updating to the current version of hledger involving multiple currencies and automated postings. It is best illustrated with an example:

= ^Expenses:Joint
    Expenses:Joint                                               *-1
    Liabilities:Joint:Bob                                       *0.5
    Liabilities:Joint:Bill                                      *0.5
    Assets:Bob:Reimbursements:Joint                            *-0.5
    Expenses:Bob                                                *0.5
    Assets:Bill:Reimbursements:Joint                           *-0.5
    Expenses:Bill                                               *0.5
    
2018-01-01 * Acme
    Expenses:Joint:Widgets                                   $100.00
    Assets:Joint:Bank                                        -£50.00

Previously, an entry such as this was processed without any problem. However, right now running hledger bal --auto results in the following error:

could not balance this transaction (real postings are off by £50.00)
2018/01/01 * Acme
    Expenses:Joint:Widgets               $100.00 @@ £50.00
    Expenses:Joint                      $-100.00 @@ £50.00
    Liabilities:Joint:Bob                 $50.00 @@ £50.00
    Liabilities:Joint:Bill                $50.00 @@ £50.00
    Assets:Bob:Reimbursements:Joint      $-50.00 @@ £50.00
    Expenses:Bob                          $50.00 @@ £50.00
    Assets:Bill:Reimbursements:Joint     $-50.00 @@ £50.00
    Expenses:Bill                         $50.00 @@ £50.00
    Assets:Joint:Bank                              £-50.00

If the currency value is entered with a @ then there is no such problem.

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Nov 13, 2018

Hi @DanMeakin. That's interesting, thanks for the report.

~/src/hledger$ hledger-1.10 -f b.j print -x 
2018/01/01 * Acme
    Expenses:Joint:Widgets    $100.00 @@ £50.00
    Assets:Joint:Bank                   £-50.00

~/src/hledger$ hledger-1.10 -f b.j print -x --auto
2018/01/01 * Acme
    Expenses:Joint:Widgets               $100.00 @ £0.5000
    Expenses:Joint                      $-100.00 @ £0.5000
    Liabilities:Joint:Bob                 $50.00 @ £0.5000
    Liabilities:Joint:Bill                $50.00 @ £0.5000
    Assets:Bob:Reimbursements:Joint      $-50.00 @ £0.5000
    Expenses:Bob                          $50.00 @ £0.5000
    Assets:Bill:Reimbursements:Joint     $-50.00 @ £0.5000
    Expenses:Bill                         $50.00 @ £0.5000
    Assets:Joint:Bank                              £-50.00
@simonmichael

This comment has been minimized.

Owner

simonmichael commented Nov 13, 2018

~/src/PLAINTEXTACCOUNTING/hledger$ cat $LEDGER_FILE
= ^Expenses:Joint
    Expenses:Joint                                               *-1
    Liabilities:Joint:Bob                                       *0.5
    Liabilities:Joint:Bill                                      *0.5
    
2018/01/01
    Expenses:Joint:Widgets                                   $100.00 @@ £50
    Assets:Joint:Bank                                        -£50.00

~/src/PLAINTEXTACCOUNTING/hledger$ hledger-1.9 print --auto
2018/01/01
    Expenses:Joint:Widgets     $100.00 @@ £50
    Assets:Joint:Bank                 £-50.00
    Expenses:Joint            $-100.00 @@ £50
    Liabilities:Joint:Bob       $50.00 @@ £50
    Liabilities:Joint:Bill      $50.00 @@ £50

~/src/PLAINTEXTACCOUNTING/hledger$ hledger-1.10 print --auto
hledger-1.10: could not balance this transaction (real postings are off by £50.00)
2018/01/01
    Expenses:Joint:Widgets     $100.00 @@ £50
    Expenses:Joint            $-100.00 @@ £50
    Liabilities:Joint:Bob       $50.00 @@ £50
    Liabilities:Joint:Bill      $50.00 @@ £50
    Assets:Joint:Bank                 £-50.00


~/src/PLAINTEXTACCOUNTING/hledger$ hledger-1.11 print --auto
hledger-1.11: "b.j" (lines 6-8)
could not balance this transaction (real postings are off by £50.00)
2018/01/01
    Expenses:Joint:Widgets     $100.00 @@ £50
    Expenses:Joint            $-100.00 @@ £50
    Liabilities:Joint:Bob       $50.00 @@ £50
    Liabilities:Joint:Bill      $50.00 @@ £50
    Assets:Joint:Bank                 £-50.00

I think: transaction modifiers with amount multipliers have never worked right if the amount has a @@ total price (and the syntax in @DanMeakin's original example is equivalent to that). hledger 1.5-1.9 generated wrongly-priced auto postings which don't balance, and didn't notice. hledger 1.10-1.11 also generate wrongly-priced auto postings but do notice that they are unbalanced.

@simonmichael

This comment has been minimized.

Owner

simonmichael commented Nov 13, 2018

Open question: is it robust to convert such total-priced amounts to unit-priced before multiplying them ? I think that'll "always" work.

simonmichael added a commit that referenced this issue Nov 14, 2018

journal: fix txn modifier multipliers with total-priced amounts (#928)
Transaction modifier multipliers have never multiplied total-priced amounts
correctly (and prior to hledger 1.10, this could generate unbalanced
transactions).

Now, the generated postings in this situation will have unit prices,
and an extra digit of display precision. This helps ensure that
the modified transaction will remain balanced. I'm not sure yet if
it's guaranteed.

simonmichael added a commit that referenced this issue Nov 14, 2018

lib: (divide|multiply)[Mixed]AmountAndPrice (#928)
Divide/multiply amounts *and* their total price, if they have one.
Helpful for keeping transactions balanced when transaction modifiers are
multiplying amounts.

simonmichael added a commit that referenced this issue Nov 14, 2018

journal: txn modifier multipliers multiply total-priced amounts (#928)
A different approach: instead of converting to unit prices and fiddling
with the display precision, just multiply the total prices by the same
multiplier (and keep them positive).

This seems a little more natural. I'm not sure if one of these will be
more robust than the other.
@simonmichael

This comment has been minimized.

Owner

simonmichael commented Nov 14, 2018

I have pushed a fix to master. The first version generated postings with unit prices and one extra decimal place, like this:

2018/01/01
    Expenses:Joint:Widgets     $100.00 @@ £50
    Expenses:Joint            $-100.00 @ £0.5
    Liabilities:Joint:Bob       $50.00 @ £0.5
    Liabilities:Joint:Bill      $50.00 @ £0.5
    Assets:Joint:Bank                 £-50.00

The latest fix just multiplies the total price by the same factor, like this:

2018/01/01
    Expenses:Joint:Widgets     $100.00 @@ £50
    Expenses:Joint            $-100.00 @@ £50
    Liabilities:Joint:Bob       $50.00 @@ £25
    Liabilities:Joint:Bill      $50.00 @@ £25
    Assets:Joint:Bank                 £-50.00

Either of these is enough to make this example work, but I'm not sure how robust they are in general. More testing and feedback needed.

@simonmichael simonmichael changed the title from Transaction involving multiple currencies and automated postings does not balance to transaction modifiers do not multiply total-priced amounts correctly Nov 14, 2018

simonmichael added a commit that referenced this issue Nov 14, 2018

@DanMeakin

This comment has been minimized.

DanMeakin commented Nov 15, 2018

Thanks very much for the very quick fixing of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment