On Fri, Nov 27, 2020 at 12:58 AM Ben Blount <[email protected]> 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.) On Thu, Nov 26, 2020 at 9:10 PM Martin Blais <[email protected]> wrote: > >> 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, >> >> >> On Tue, Nov 17, 2020 at 12:44 PM Ben Blount <[email protected]> wrote: >> >>> First draft on the clarification doc is ready for review. >>> <https://docs.google.com/document/d/1rclTmxJzq-4Na1UPY0XfS9Yzuv_SuHpdfHtabNsUH0E/edit#> >>> 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. >>> >>> On Mon, Nov 16, 2020 at 9:53 PM Martin Blais <[email protected]> wrote: >>> >>>> On Tue, Nov 17, 2020 at 12:42 AM Martin Blais <[email protected]> wrote: >>>> >>>>> On Tue, Nov 17, 2020 at 12:21 AM Ben Blount <[email protected]> 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. >>>> >>>> >>>> >>>>> >>>>> Looking fwd, >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> On Mon, Nov 16, 2020 at 6:48 PM Martin Blais <[email protected]> wrote: >>>>>> >>>>>>> On Mon, Nov 16, 2020 at 9:02 PM Ben Blount <[email protected]> wrote: >>>>>>> >>>>>>>> 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 >>>>>>>> <https://groups.google.com/g/beancount/c/SP5KeksHbCk/m/aIMfFDMRBQAJ> >>>>>>>> been >>>>>>>> <https://groups.google.com/g/beancount/c/72PN-d-je40/m/lHQZf6O0CQAJ> >>>>>>>> asked >>>>>>>> >>>>>>>> <https://groups.google.com/g/beancount/c/dOrCPTbHZKY/m/vhPipQaVAQAJ> 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. >>>>>>>> >>>>>>>> <https://groups.google.com/g/beancount/c/TjOnum255Ps/m/hDFkdqCaAwAJ> 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. >>>>>>>> <https://groups.google.com/g/beancount/c/SP5KeksHbCk/m/bOXNBFOjAwAJ> >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>> >>>>>>> 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 [email protected]. >>>>>>>> To view this discussion on the web visit >>>>>>>> https://groups.google.com/d/msgid/beancount/CACGEkZvQ_bFqf%3DKsriEC5qMJOj5r0Oh8c-bF6Cj-hT%3D2eAS7Gg%40mail.gmail.com >>>>>>>> <https://groups.google.com/d/msgid/beancount/CACGEkZvQ_bFqf%3DKsriEC5qMJOj5r0Oh8c-bF6Cj-hT%3D2eAS7Gg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>>> . >>>>>>>> >>>>>>> -- >>>>>>> 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 >>>>>>> [email protected]. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/beancount/CAK21%2BhNStTJ7QkSvKWJpYWDTwcUc%3DggROujgPB72_Ge0HaFW6A%40mail.gmail.com >>>>>>> <https://groups.google.com/d/msgid/beancount/CAK21%2BhNStTJ7QkSvKWJpYWDTwcUc%3DggROujgPB72_Ge0HaFW6A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >>>>>> 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 [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/d/msgid/beancount/CACGEkZtDSSdAFQKJ-afk_gKmwi%3DSDP2tfHw5zVycsY1G5RKx0g%40mail.gmail.com >>>>>> <https://groups.google.com/d/msgid/beancount/CACGEkZtDSSdAFQKJ-afk_gKmwi%3DSDP2tfHw5zVycsY1G5RKx0g%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>> -- >>>> 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 >>>> [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/beancount/CAK21%2BhPyW6r4xyXvdK4ye6nNVsUcLpwd5h6oFCHDSGvXiLFZeQ%40mail.gmail.com >>>> <https://groups.google.com/d/msgid/beancount/CAK21%2BhPyW6r4xyXvdK4ye6nNVsUcLpwd5h6oFCHDSGvXiLFZeQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/beancount/CAK21%2BhPkKtTXd4%3DLZmkeB8Bc1PZ5BNx6xHfHROVUdLp7Q_SHHw%40mail.gmail.com.
