simonmichael / hledger

Robust, fast, intuitive plain text accounting tool with CLI, TUI and web interfaces.
https://hledger.org
GNU General Public License v3.0
3.05k stars 320 forks source link

incorrect prices for "daily" roi #2199

Open aragaer opened 6 months ago

aragaer commented 6 months ago

Slightly modified snake oil example:

P 2019-09-30 SNKOIL $1.075

2019-12-20 Investing in Snake Oil
 assets:cash
 investment:snake oil   100 SNKOIL @@ $107.5

P 2019-12-23  SNKOIL $1.08

2019-12-23 Fear of missing out
 assets:cash 
 investment:snake oil   83.33 SNKOIL @@ $90 ; $90/$1.08 = 83.33

;P 2019-12-23  SNKOIL $1.09  ; -- this line affect the 2019-12-22 ROI result

; Recording the growth of Snake Oil
P 2019-12-24  SNKOIL $1.1

2019-12-25 check
 investment:snake oil   0 = 183.33 SNKOIL

The issue here is that there is another (commented in the example) price directive for 2019-12-23. When doing a per-day roi I'm expecting that the final price for a date would be the first price of the next day, however the last price is used instead. Adding a price directive to reflect that the price changed after the additional purchase affects the calculated roi of previous day.

Apparently it is not directly related to roi, just the general case of using the next day as "end" of a period and using the last price when multiple directives are given for the same date.

simonmichael commented 5 months ago

Thanks @aragaer, I imagine @adept will have thoughts on this at some point.

adept commented 5 months ago

Roi does not interact with pricing directives directly, it just asks pricing functions to price things. So it would be very hard to change how Roi behaves here without changing the behaviour of the rest of the hledger.

I think this is the case of "multiple pricing directives per day are generally not (very well?) supported in hledger"

simonmichael commented 5 months ago

Ah, yes. I guess that is probably what you're seeing. Valuation (pricing) in hledger uses a single P directive per day, the last one parsed if there is more than one, I am guessing. I found slight mention of it at https://hledger.org/dev/hledger.html#finding-market-price .

simonmichael commented 5 months ago

@aragaer did that explain it ?

aragaer commented 5 months ago

Yes, this is exactly how I understood it. The question is -- should it be fixed? Is it worth implementing a different valuation request which will use first price instead, so that it is used for "end" valuations?

simonmichael commented 5 months ago

I think when there are multiple similar directives, generally the last one wins. Similarly to when there are multiple similar command line options, the last one wins. So it sounds like the current behaviour is at least consistent with hledger's usual way of doing things ?

aragaer commented 5 months ago

When calculating roi for certain period we want the price at the end of period. Current implementation is to use the price for the first day after that period, which means that first directive would be more appropriate. Unfortunately there is currently no way to specify "end price".

adept commented 5 months ago

I dont this that this lines up with how the pricing code works, btw.

But regardless, I think that support of more than one price per day would require a lot of forward planning, cause opportunities for tricky edge-cases would expand significantly.

Nevermind first and last price of the day, what about having 15 price directives for a given day, interspersed with transactions? What if those prices (and transactions) come from different files via include directives, and include graph has "diamonds"? Etc etc