Average cost booking

[Ben Blount]

First off, thank you so much for such a great tool!

One challenge I haven't been able to solve yet with beancount is tracking basis for my US 401k -  average cost booking.

I did quite a bit of digging into the code, TODOs, and the mailing list. I actually implemented average cost booking on augmentation and reduction as a plugin, but it's not possible with just a plugin due to booking running before plugins do (makes sense- having resolved lots is very useful).

My summary from investigations:

• It's been asked a fair bit on the mailing list.
• The best workaround currently is to use the "NONE" booking method and manually summarize it everywhere you need to. Martin has a decent workaround here. I wrote a plugin to automate some parts of Martin's 
• Since it touches the heart of the balance update mechanism it could break many things if done without care. Martin previously (understandably) would rather not accept it as a contribution.

Martin: Do you still feel the same way? I am wondering if perhaps you'd want contributions of unit tests for it? I'd love to do anything I can to help get AVERAGE booking going, my 401k has a lot of history and contributions so it's a big chore to track.

Thanks!

[b...@bben.us]

Also: is any of this affected by v3? It doesn't look like it to me based on my limited understanding.

[Martin Blais]

Yes, nothing much has changed. It's possible to do, I just think it might create a lot of unexpected work, but it's possible. FWIW, I want that feature too!  If you're willing to (a) include tests covering most corner cases for the code you're adding, (b) finish coverage of the feature to the extent of not leaving too many loose ends and handle most of the corner cases (will take more time than one might think IMHO, but what do I know about your level of enthusiasm for this problem?) and (c) support it when people report things that aren't working, at least until it's really stable, I'm happy to review PRs. You can start the work in a branch too if you prefer.

There's already a method for it, and you don't even have to change the syntax, start here:

https://github.com/beancount/beancount/blob/master/beancount/parser/booking_method.py#L173

The way I think it would best be implemented is that on an augmentation or reduction to an account marked as AVERAGE booking method, merge all the lots from that account in each commodity to a per-commodity lot with the average cost. Implementing that naively is not difficult. Optionally, you could trigger the merging only on reduction (would work too, less rounding errors, but less consistency). But... find a way to handle rounding errors gracefully.  Also, the resulting costs will be with long fractions (not rounded)... or will they be rounded? If the costs aren't rounded, are users going to be able to specify a cost value to reduce?  How would that match against the number they input if the costs have 20 fractional digits? (maybe won't be that important since there's only a single position to reduce from anyway and one could always elide the cost to match against it? Is that true? I'd like to see it in tests, maybe that works well). How about the case where there are multiple commodities in the same account? Or the same commodity priced in different currencies, what should occur? People will do these things. And how to deal with the case that would result in a negative cost (e.g. inventory = 100 HOOL {60 USD}, + reduction of -11 HOOL {600 USD}, what should happen? Raise an error? Or allow negative cost? Okay, so say you decide to issue an error. What is this occurs in a transient situation, e.g., that state only occurs as a temporary state of an inventory while processing a jumble of reductions and augmentations from the same transaction, the end state of which would be legitimate?). My intuition based on the past booking features I've implemented is that a lot of stuff like that will come up as you implement it, and more that we aren't thinking about yet, but it's definitely worth a shot. You can add tests for all these corner cases. If the issues I raised in this paragraph sound new, I think the right way to go about this is to try it out isolating each of these corner cases with dedicated unit tests for each, and carefully document each of the cases in a shared doc with examples as they come up, and solicit feedback and/or find creative solutions to them, and document them, as liberal comments in the code or in that doc.

As for v3, if it works well, I would just convert that to the equivalent C++ code, like the rest of the core (slowly underway).

Thanks!

--
You received this message because you are subscribed to the Google Groups "Beancount" group.
To unsubscribe from this group and stop receiving emails from it, send an email to beancount+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CACGEkZvQ_bFqf%3DKsriEC5qMJOj5r0Oh8c-bF6Cj-hT%3D2eAS7Gg%40mail.gmail.com.

[Ben Blount]

Great, glad to hear Martin!

I'll start with a doc similar to your docs that outlines the implementation decisions to be made, offering options for each. Once we have consensus on those and if it still looks doable for me, I'll start building out the testcases in a branch on my own repo.

I saw some references to previous testcases written in a 'booking' branch. I don't see that branch anymore, perhaps it was on the old git provider. Do you know where these are / are they worth reusing?

You received this message because you are subscribed to a topic in the Google Groups "Beancount" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/beancount/1RRUo04hNbI/unsubscribe.
To unsubscribe from this group and all its topics, send an email to beancount+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CAK21%2BhNStTJ7QkSvKWJpYWDTwcUc%3DggROujgPB72_Ge0HaFW6A%40mail.gmail.com.

[Martin Blais]

On Tue, Nov 17, 2020 at 12:21 AM Ben Blount <b...@bben.us> wrote:

Great, glad to hear Martin!

I'll start with a doc similar to your docs that outlines the implementation decisions to be made, offering options for each. Once we have consensus on those and if it still looks doable for me, I'll start building out the testcases in a branch on my own repo.

SGTM. I think a good way is to write out small scenarios in unit tests illustrating those cases I enumerated and you'll think of more. The loader.load_doc() helper and the corresponding parse_string() for expected outputs make it really easy to do that without having to write any real code. 

I saw some references to previous testcases written in a 'booking' branch. I don't see that branch anymore, perhaps it was on the old git provider. Do you know where these are / are they worth reusing?

I... can't remember. It's been too long. I ported the "booking_fixes" branch, maybe that's the one.

Looking fwd,

 

 

To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CACGEkZtDSSdAFQKJ-afk_gKmwi%3DSDP2tfHw5zVycsY1G5RKx0g%40mail.gmail.com.

[Martin Blais]

On Tue, Nov 17, 2020 at 12:42 AM Martin Blais <bl...@furius.ca> wrote:

On Tue, Nov 17, 2020 at 12:21 AM Ben Blount <b...@bben.us> wrote:

Great, glad to hear Martin!

I'll start with a doc similar to your docs that outlines the implementation decisions to be made, offering options for each. Once we have consensus on those and if it still looks doable for me, I'll start building out the testcases in a branch on my own repo.

SGTM. I think a good way is to write out small scenarios in unit tests illustrating those cases I enumerated and you'll think of more. The loader.load_doc() helper and the corresponding parse_string() for expected outputs make it really easy to do that without having to write any real code. 

I saw some references to previous testcases written in a 'booking' branch. I don't see that branch anymore, perhaps it was on the old git provider. Do you know where these are / are they worth reusing?

I... can't remember. It's been too long. I ported the "booking_fixes" branch, maybe that's the one.

Actually no, there's nothing interesting in that branch. I had started to fix https://github.com/beancount/beancount/issues/167 and https://github.com/beancount/beancount/issues/168 in booking_fixes, unrelated.

[Ben Blount]

First draft on the clarification doc is ready for review.

In short, I suggest we start with the feature in the most restrictive state, where multiple cost currencies, and specifying cost basis manually isn't allowed for accounts using the AVERAGE booking method. AVERAGE cost booking is still immensely useful even if these cornercases are not supported. Users that require them can still use the current workaround of NONE booking with manual aggregations.

My biggest open question is around inside-transaction posting ordering, and if beancount already has conventions for how to handle that. I also need to think through http://furius.ca/beancount/doc/self-reductions a bit more to see if that will affect this.

I hit most of the edge cases you mentioned when building my average rebooking plugin, but it's quite possible that I'll hit more once I get into the testcases/implementation. I'll move those into the doc for discussion / posterity as they come up.

To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CAK21%2BhPyW6r4xyXvdK4ye6nNVsUcLpwd5h6oFCHDSGvXiLFZeQ%40mail.gmail.com.

[Martin Blais]

Reviewed. I think you got to most of the issues (see comments).

Totally fine starting with a constrained version.

The issue of reducing within a transaction is a thorny one, but I think it'll be fine to just reduce against the inventory that comes before the transaction.

Thank you,

[Ben Blount]

Thanks for collaborating with me Martin!

I have the 'trivial' version working and have tested it on my personal ledger. I added several testcases, but need more before I put up a PR. The current implementation doesn't currently rebook at average on augmentations: that requires tweaking the code in booking_full. The booking methods are /only/ invoked now to disambiguate lots in reductions.

I'll have some time this weekend and hope to have a PR ready soon.

[Martin Blais]

On Fri, Nov 27, 2020 at 12:58 AM Ben Blount <b...@bben.us> wrote:

Thanks for collaborating with me Martin!

I have the 'trivial' version working and have tested it on my personal ledger. I added several testcases, but need more before I put up a PR. The current implementation doesn't currently rebook at average on augmentations: that requires tweaking the code in booking_full. The booking methods are /only/ invoked now to disambiguate lots in reductions.

I see, that makes sense. IIRC the booking code only triggers on reductions.

 

I'll have some time this weekend and hope to have a PR ready soon.

SGTM, thank you.

(Cycles are limited, and I'm smack in the middle of a rewrite of the parser purely in C++... please send small, incremental PRs as much as possible.)