Repository navigation
--infer-market-prices infers negative prices #1870
Description
With this entry:
2022-01-01
a A -1
b B 1 @@ A 1
hledger 1.25 and 1.26 infer the same market price:
$ hledger-1.25 prices --infer-market-prices
P 2022-01-01 B A 1.0
$ hledger-1.26 prices --infer-market-prices
P 2022-01-01 B A 1.0
and produce the same valuation:
$ hledger-1.25 print --infer-market-prices -V
2022-01-01
a A -1
b A 1
$ hledger-1.26 print --infer-market-prices -V
2022-01-01
a A -1
b A 1
But with this entry, which is odd but currently considered legal and balanced, since the second posting's negatives cancel out:
2022-01-01
a A -1
b B -1 @@ A -1
hledger 1.25 inferred the market price correctly but produced an incorrect valuation (note both amounts are negative):
$ hledger-1.25 prices --infer-market-prices
P 2022-01-01 B A 1.0
$ hledger-1.25 print --infer-market-prices -V
2022-01-01
a A -1
b A -1
hledger 1.26 fixed the valuation, but now incorrectly infers a negative market price:
$ hledger-1.26 prices --infer-market-prices
P 2022-01-01 B A -1.0
$ hledger-1.26 print --infer-market-prices -V
2022-01-01
a A -1
b A 1
I had several of those odd double negative entries (generated by csv rules, unknowingly), and this caused hledger 1.26 to report an incorrect balance in one of my reports. A minimal example:
$ hledger-1.25 bal b -N -X A --infer-market-prices
A -1 b
$ hledger-1.26 bal b -N -X A --infer-market-prices
A 1 b
Activity
[-]--infer-market-prices infers negative prices in a new way[/-][+]--infer-market-prices infers negative prices[/+]Xitian9 commentedon Jun 7, 2022
Ugh, the total price for negative values is extremely counter-intuitive. But looking at this, I don't see why this isn't a negative price. If you think the correct valuation of
B -1 @@ A -1isA 1, then we have thatB -1is equivalent toA 1, which is a negative price.But regardless of how this particular issue gets resolved, we really need to make this more intuitive. I was the one who wrote this and I can't get my head straight about what the correct behaviour should be.
Xitian9 commentedon Jun 7, 2022
Here is a proposed test suite. Here are the properties I believe we should satisfy.
--costshould be equivalent to--infer-market-prices --value=then.Based on my testing for hledger-1.26, all these hold except (3) for the transactions on 2022-01-01.These pass all these tests, provided you're sure to use--value=thento get the valuation on the date of the transaction. Let me know if you agree.simonmichael commentedon Jun 9, 2022
Thanks for the examples and notes. More: here are the market prices inferred from the above by 1.25 and 1.26:
Questions: should we allow or disallow negative transaction prices and negative market prices, what do they mean, and if allowed how exactly should they work ?
I have not used negative prices or seen them used intentionally.
Our docs do not mention them (and Ledger's and Beancount's probably don't either). Here are some related places:
1 -> item 3 describes entries where conversion price is left implicit. hledger always rejects such an entry if both amounts have the same sign, so that's one place where we enforce that the inferred price is positive.
One way to simplify would be to disallow negative transaction prices and negative market prices. Did we discuss and reject this in the past ?
simonmichael commentedon Jun 9, 2022
Ledger currently disallows negative transaction prices (costs), but allows negative market prices. Beancount does the same.
simonmichael commentedon Jun 9, 2022
I think that won't always be true since a P directive on the same day can override an inferred market price.
Xitian9 commentedon Jun 9, 2022
True, I was referring to the more limited case in this example, where there were no price directives. To be careful, we should also expand those transactions so they're all on different days.
Xitian9 commentedon Jun 9, 2022
Internally we have to allow negative transaction prices, as that might be the result of summing two positive total prices:
$50 @@ €40 + (-$60 @@ €30) == -$10 @@ -€10. In words, if I buy $50 at a cost of €40, and then sell $60 and receive €30 for them, the overall result is that I've lost $10 and €10; that's a negative price.We could disallow negative prices in journals, but I can see this running into problems if any internal negative prices leak out into a journal, for example with the
closeorprintcommands.simonmichael commentedon Jun 10, 2022
Here's real world https://en.wikipedia.org/wiki/Negative_pricing, also.
simonmichael commentedon Jun 10, 2022
Aside: above,
pricesshows the prices inferred from @@ with a .0, it would be nice to hide that / show all prices with consistent format.simonmichael commentedon Jun 10, 2022
After discussion and re-reading everything, I'm starting to get used to the idea that 1.26 is handling my and your examples correctly and all that may be needed is more documentation about our support for negative prices and how that works with @ and @@.
Xitian9 commentedon Jun 10, 2022
Agreed.
15 remaining items
tanelihuuskonen commentedon Oct 25, 2022
Glad to hear we agree in general.
Adding an option to always show the true sign of a total cost in output would be an easy first step. As for input, I see three basic possibilities:
Option (3), inferring the correct sign when the input is ambiguous, is possible if we know the sign of the unit price, which can be assumed to be positive unless explicitly declared otherwise, maybe on a per-commodity basis. However, I'm not sure it's worth implementing, as there may be room for subtle bugs and hard-to-find user errors. Just adding a command-line option for (2) is enough for converting existing journals back and forth.
simonmichael commentedon Oct 25, 2022
More great examples and discussion - thanks!
The current usage of @@ has a certain logic when you read it in the right way. But right now I too agree the above looks more principled, therefore easier to keep straight.
I must still be slightly confused though: @tanelihuuskonen's examples look similar to @Xitian9's june 6th proposed test suite above, but the latter works already with hledger 1.26. I expect I'll see the difference in a minute.
An optional different behaviour, or even change of default behaviour, is not impossible. We should think about whether the reduction in unclarity/confusion would outweigh the new complexity, confusion and churn it would bring.
When someone gets the chance, it would be interesting to also see examples/tests showing the result of --infer-market-price on each of these example transactions, another aspect of this (and the original trigger for this issue).
tanelihuuskonen commentedon Oct 26, 2022
@simonmichael : Yes, I agree.
My examples differ from those of @Xitian9 in the cases where you sell a commodity (or pay to get rid of it) and record the total price. I added a dummy equity:error account to those ones to make hledger 1.27.1 accept them. They correspond to @Xitian9 's examples with total prices on 2022-01-01 and 2022-01-03.
simonmichael commentedon Dec 7, 2022
This has had no impact in my daily usage [since June, I mean; it did impact me then] so I have been unmotivated. Its status as a bug and a regression has felt a bit variable. But it's not good too leave it open so long when it is marked as a regression. Just noting that this issue needs a champion.
simonmichael commentedon Dec 14, 2022
Here are the tests I'm adding (@Xitian9's examples):
They pass since hledger 1.26. If anyone wants to push @tanelihuuskonen's proposal forward, the next step would be to raise it on the mail list and try to gather support / estimate impact. We could build a test binary and ask people if it breaks with their data. I'm not too keen on adding an option but I suppose we would have to, to ease the transition, or provide a conversion tool.
dev: tests demonstrating signed costs and market price inference (#1870)
doc: cli: document --infer-market-prices with signed costs (#1870)
simonmichael commentedon Dec 14, 2022
Added to manual at
https://hledger.org/dev/hledger.html#--infer-market-prices-market-prices-from-transactions
simonmichael commentedon Dec 14, 2022
Closing this as a documentation bug for 1.26's change in market price inference, not a regression. If more happens we can reopen or start a new issue.