support balance resetting and make assertion semantics same with ledger #129

Closed
wants to merge 2 commits into
from

Projects

None yet

5 participants

@xiaoruoruo
Contributor

To support balance resetting, we need to keep track of "current" account balances. The algorithm for balance assertion/assignment/resetting is basically the same: derive the amount from the assertion, and compare it to the actual amount, if any.

If there are multiple assertion postings for the same account in one transaction, it's not always possible to derive&update posting by posting. So when this happens, I don't update the balances mapping until all postings are checked for a transaction. This seems to be the semantics of ledger 3.0 also. So the tests are also updated/modified to reflect these situation.

@simonmichael
Owner

Hi Xinruo,

thank you very much!

I've done a first read-through. I like that you've added functionality
and reduced the code.

It's nice to separate concerns where possible for easier code
comprehension. Is it necessary to do

  • checking that a transaction's postings balance, and filling in
    missing amounts
  • checking historical balance assertions, and resetting account
    balances

all at once ? Can these be separated at all ?

Do you know if there's any performance impact ? The "quickbench" and
"samplejournals" make targets may be useful.

A more serious concern is that I find this ledger feature as currently
designed pretty confusing to use and to understand the semantics of.
I've discussed this on #ledger in the past, but don't yet remember all
the details (and there's no channel log). It feels like too many
interlocking meanings of the = symbol and missing amounts. Maybe
balance assertion and assignment should have different symbols ?

I'll try it here and we can discuss more. I'd welcome any other
opinions.

@simonmichael
Owner

(Comment from Thomas, don't know why it has my name on it --sm)

It would be a lot easier to understand if bug report / fix report came
with pastes of hledger usage, or screencaps of hledger web uses, that
show behavior of program before and after, from user perspective.

Can this be provided?

@simonmichael
Owner
simonmichael commented Jul 27, 2013 edited

Yes this can be confusing, here's some more context:

and for some examples there's the included functional tests.

Balance assertions are desirable to provide extra guarantees about the correctness of your data, since *ledger is so permissive. They help you avoid messing up balances while editing.

Balance assignment/resetting is something most people would rarely use. I can see it is occasionally useful to be able to specify an account's balance in absolute terms though.

@xiaoruoruo
Contributor

Hi Simon,

Thanks for the reply.

Yes I also don't like that those functionality are intertwined together and hard to understand. But this is the best that I can think of, algorithm-wise. I know this is more "procedural" style instead of functional style. If there are any suggestion on how to improve the code I would be happy to refactor.

As of the feature, one can refer to the Simon's links above. Especially the examples in http://ledger-cli.org/3.0/doc/ledger3.html#Balance-verification .

From the user point of view, if one doesn't use any balance verification, this patch changed nothing. This patch only changes behavior when there are multiple postings of the same account in the same transaction. For example:

2013/1/1  before
  a    $2  = $2
  b   $-1  = $-1
  b   $-1  = $-2

After:

2013/1/1  after
  a    $2  = $2
  b   $-1  = $-1
  b   $-1  = $-1

Oh and of course this patch brings the balance resetting feature described in ledger doc section 5.10.3.

I don't know what others are thinking about this feature, but it makes sense to me and I would be frequently using them. From my point of view, the = symbol maybe a bit overloaded, but it serves as the same good purpose: it's telling ledger that this account should have this balance to this point. This remains true for all three cases: assertion, assignment, resetting. Difference is the degree of intelligence ledger is applying. This is a bit similar to haskell's type inference. If there is amount/type-signature, please verify if it's the case. If there's missing amount/type-signature, use it as the golden balance/type to infer other postings' amounts/types.

Here comes benchmark. Compiled using GHC 7.6.3 and cabal install.

+-------------------------------------------++----------------+------------------+--------------+-------------+---------------------+
|                                           || hledger-0.19.3 | hledger-0.20.0.1 | hledger-0.21 | hledger-dev | ledger-3.0-20120518 |
+===========================================++================+==================+==============+=============+=====================+
| -f data/100x100x10.journal     balance    ||           0.05 |             0.06 |         0.06 |        0.06 |                0.02 |
| -f data/1000x1000x10.journal   balance    ||           0.33 |             0.43 |         0.45 |        0.44 |                0.08 |
| -f data/10000x1000x10.journal  balance    ||           2.33 |             3.43 |         3.48 |        3.44 |                0.29 |
| -f data/10000x1000x10.journal  balance aa ||           3.14 |             4.21 |         4.28 |        4.22 |                0.27 |
| -f data/10000x10000x10.journal balance    ||           5.14 |             6.46 |         6.17 |        6.10 |                0.63 |
| -f data/100x100x10.journal     register   ||           0.06 |             0.09 |         0.08 |        0.08 |                0.06 |
| -f data/1000x1000x10.journal   register   ||           0.45 |             0.61 |         0.62 |        0.61 |                0.48 |
| -f data/100x100x10.journal     print      ||           0.05 |             0.06 |         0.06 |        0.06 |                0.02 |
| -f data/1000x1000x10.journal   print      ||           0.31 |             0.43 |         0.44 |        0.43 |                0.10 |
| -f data/10000x1000x10.journal  print      ||           3.39 |             4.53 |         4.15 |        4.09 |                0.54 |
+-------------------------------------------++----------------+------------------+--------------+-------------+---------------------+

Here hledger-0.21 is master branch. hledger-dev is after applied this patch. The results suggest dev performs slightly faster than 0.21 and 0.20.

@xiaoruoruo
Contributor

I forgot to explain why the logics cannot be separated. Consider this example:

2013/1/1
  a    $1  =$1
  b        =-$1

2013/1/2
  a        =$2
  b             ; =-$2

2013/1/3
  a             ; =-$2
  b        =$2

If you try to group the postings by account, then you won't be able to derive
all amounts for account b. To derive the amount for balance resetting, it must
go through the transactions and keep the balances of all account.

The rest are just associated computation. By filling in the missing amounts, we
can continue the book keeping. Balance assertions and assignments can also be
checked along the way.

@tphyahoo

For what it's worth, I don't use balance assertions but now that I know what they are I think I may try them out. They seem very useful.

@simonmichael
Owner

How can

2013/1/1 after
a $2 = $2
b $-1 = $-1
b $-1 = $-1

make sense ? As a user, it seems arithmetically wrong.

@xiaoruoruo
Contributor

This issue is staying a bit long. Please close it if you think it's a minor feature. Or advise how I should proceed. Thanks.

Just FYI, here's how ledger-cli treats this.

$ cat a.ledger 
2013/1/1  before
  a    $2  = $2
  b   $-1  = $-1
  b   $-1  = $-2

$ ledger -f a.ledger bal
While parsing file "a.ledger", line 4: 
While parsing posting:
  b   $-1  = $-2
             ^^^
Error: Balance assertion off by $-1

$ cat b.ledger 
2013/1/1  before
  a    $2  = $2
  b   $-1  = $-1
  b   $-1  = $-1

$ ledger -f b.ledger bal
                  $2  a
                 $-2  b
--------------------
                   0
@simonmichael
Owner

Hi.. as I said above, I don't see how the ledger behaviour makes sense here. It doesn't add up. How would you convince a new user that ledger's behaviour makes more sense than hledger's here ?

@xiaoruoruo
Contributor

So do you think a.ledger makes sense? In my opinion both cases don't really make sense. Users should probably avoid balance assertions in transactions with multiple postings to the same account.

@simonmichael
Owner

Yes I think a.ledger makes sense at least in this simple case. Both accounts start at 0. After posting $-1 to b, its balance is $-1. After posting $-1 to b again, its balance is $-2. I don't see how b.ledger makes sense.

@simonmichael
Owner

Closing, please reopen to discuss further if needed.

@johannesgerer
Contributor

Update

ledger seems to have updates its semantics to be more consistent:

@simonmichael your example from above now is not considered balanced. The transaction accepted by both hledger and ledger now looks like this:

2013/1/1 after
      a      $2 = $2
      b      $-1 = $-1
      b      $-1 = $-2

This makes more sense.

The semantics

For a given transaction:

  1. First handle all postings with assertions and/or amounts:
    1.1. Posting with amount: Add the amount to the account balance.
    1.1.1 ... and assertion: Check Assertion == Account Balance
    1.2. Posting without amount: infer Amount = Assertion - Account Balance
  2. Then handle a posting without amount or assertion: infer Amount = - Transaction Balance

Why this feature is SO IMPORTANT:

I use hledger to track my personal finances including my wallet and other cash accounts that are frequently changing. Tracking all cash expenses up to a certain point of time can be hard due to forgetfulness etc, and because things like cash withdrawals have some delay until they find their way into my ledger through my bank statements. This is why at any given point I can easliy count my wallet at hand, but the ledger balance might not yet be up to date. With an "assigment" posting this situation can be handled extremely easy and fail safe. Not having this feature means constant mundane and error prone manual work.
Does this make any sense?

The pull request

I will send an updated pull request. As the parsing and data types already support assertions with missing amounts, there should be only little change needed.

What do I need to provide to help/guarantee a fast merge on your end?

@ony
Collaborator
ony commented Dec 6, 2016

@johannesgerer I'm happy to hear that this functionality may be merged into hledger.
But please consider next example

2016/12/05 open
    a  = $10
    b

2016/12/06 multipost
    a  = $2
    a  -$2  ; May add assertion like: = $0
    c  ; <-- $8 or $6?

I'm not sure that you can re-order postings. Either you should reject such kind of transactions or you should process in order of writing, I believe.

@simonmichael
Owner
simonmichael commented Dec 6, 2016 edited

@johannesgerer thanks for the update. I'm glad to hear about the fix in Ledger. I'll be very happy to review and test your patch.

@simonmichael
Owner

(Or your update of @xiaoruoruo's patch, if that's what it is).

FYI #157 is a semi-duplicate issue with some more recent design discussion.

@simonmichael
Owner

Also related: #288. This feature seems to be quite popular.

@ony
ony reviewed Dec 7, 2016 edited View changes

It looks like with last merge commit to assignment you overwrite your prev commit for balance reset. If you want to start from scratch consider simply hard reset your branch to the latest changes in hledger.
Sorry by some reason github said "New changes since you last viewed" on pull request.

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