From e035730afb6f488fceca70b0d343df5995ce4272 Mon Sep 17 00:00:00 2001 From: Simon Michael Date: Wed, 8 Nov 2023 12:48:58 -0800 Subject: [PATCH] imp: roi: limit large decimals to 8 digits by default (precisiongeddon) With valuation now preserving more decimal digits, roi could show excessively precise decimals if there was no known display precision for the valuation commodity. Now in that situation it limits the precision to a maximum of 8 digits. --- hledger-lib/Hledger/Data/Amount.hs | 19 ++++++++++++++ hledger-lib/Hledger/Data/Valuation.hs | 25 ++++++++++++------- hledger/Hledger/Cli/Commands/Roi.hs | 10 ++++++-- hledger/test/roi.test | 36 ++++++++++++++++----------- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/hledger-lib/Hledger/Data/Amount.hs b/hledger-lib/Hledger/Data/Amount.hs index 279788783..6ceca03d7 100644 --- a/hledger-lib/Hledger/Data/Amount.hs +++ b/hledger-lib/Hledger/Data/Amount.hs @@ -91,6 +91,7 @@ module Hledger.Data.Amount ( showAmountWithoutPrice, amountSetPrecision, amountSetPrecisionMin, + amountSetPrecisionMax, withPrecision, amountSetFullPrecision, amountSetFullPrecisionOr, @@ -154,6 +155,8 @@ module Hledger.Data.Amount ( wbUnpack, mixedAmountSetPrecision, mixedAmountSetFullPrecision, + mixedAmountSetPrecisionMin, + mixedAmountSetPrecisionMax, -- * misc. tests_Amount @@ -395,6 +398,12 @@ amountSetPrecisionMin :: Word8 -> Amount -> Amount amountSetPrecisionMin minp a = amountSetPrecision p a where p = Precision $ max minp (amountDisplayPrecision a) +-- | Ensure an amount's display precision is at most the given maximum precision. +-- Always sets an explicit Precision. +amountSetPrecisionMax :: Word8 -> Amount -> Amount +amountSetPrecisionMax maxp a = amountSetPrecision p a + where p = Precision $ min maxp (amountDisplayPrecision a) + -- | Increase an amount's display precision, if needed, to enough decimal places -- to show it exactly (showing all significant decimal digits, without trailing zeros). -- If the amount's display precision is unset, it will be treated as precision 0. @@ -1189,6 +1198,16 @@ mixedAmountSetPrecision p = mapMixedAmountUnsafe (amountSetPrecision p) mixedAmountSetFullPrecision :: MixedAmount -> MixedAmount mixedAmountSetFullPrecision = mapMixedAmountUnsafe amountSetFullPrecision +-- | In each component amount, ensure the display precision is at least the given value. +-- Makes all amounts have an explicit Precision. +mixedAmountSetPrecisionMin :: Word8 -> MixedAmount -> MixedAmount +mixedAmountSetPrecisionMin p = mapMixedAmountUnsafe (amountSetPrecisionMin p) + +-- | In each component amount, ensure the display precision is at most the given value. +-- Makes all amounts have an explicit Precision. +mixedAmountSetPrecisionMax :: Word8 -> MixedAmount -> MixedAmount +mixedAmountSetPrecisionMax p = mapMixedAmountUnsafe (amountSetPrecisionMax p) + -- | Remove all prices from a MixedAmount. mixedAmountStripPrices :: MixedAmount -> MixedAmount mixedAmountStripPrices (Mixed ma) = diff --git a/hledger-lib/Hledger/Data/Valuation.hs b/hledger-lib/Hledger/Data/Valuation.hs index c18bc6cc6..b52502498 100644 --- a/hledger-lib/Hledger/Data/Valuation.hs +++ b/hledger-lib/Hledger/Data/Valuation.hs @@ -25,6 +25,7 @@ module Hledger.Data.Valuation ( ,marketPriceReverse ,priceDirectiveToMarketPrice ,amountPriceDirectiveFromCost + ,valuationTypeValuationCommodity -- ,priceLookup ,tests_Valuation ) @@ -68,6 +69,14 @@ data ValuationType = | AtDate Day (Maybe CommoditySymbol) -- ^ convert to default or given valuation commodity, using market prices on some date deriving (Show,Eq) +valuationTypeValuationCommodity :: ValuationType -> Maybe CommoditySymbol +valuationTypeValuationCommodity = \case + AtThen (Just c) -> Just c + AtEnd (Just c) -> Just c + AtNow (Just c) -> Just c + AtDate _ (Just c) -> Just c + _ -> Nothing + -- | A price oracle is a magic memoising function that efficiently -- looks up market prices (exchange rates) from one commodity to -- another (or if unspecified, to a default valuation commodity) on a @@ -193,17 +202,15 @@ amountValueAtDate priceoracle styles mto d a = -- Manage style and precision of the new amount. Initially: -- rate is a Decimal with the internal precision of the original market price declaration. -- aquantity is a Decimal with a's internal precision. - -- The resulting internal precision will be larger than both (their sum ?). - -- The display precision will be that of nullamt (0). - -- Now apply the standard display style for comm + -- The calculated value's internal precision may be different from these. + -- Its display precision will be that of nullamt (0). + -- Now apply the standard display style for comm (if there is one) & styleAmounts styles - -- and set the display precision to rate's internal precision - -- (unnormalised - don't strip trailing zeros) - -- & amountSetPrecision (Precision $ decimalPlaces rate) - & amountSetFullPrecisionOr Nothing -- (Just defaultMaxPrecision) + -- set the display precision to match the internal precision (showing all digits), + -- unnormalised (don't strip trailing zeros); + -- but if it looks like an infinite decimal, limit the precision to 8. + & amountSetFullPrecisionOr Nothing & dbg9With (lbl "calculated value".showAmount) - -- & dbg9With (lbl "precision of value".show.amountDisplayPrecision) - -- see also print-styles.test, valuation2.test -- | Calculate the gain of each component amount, that is the difference -- between the valued amount and the value of the cost basis (see diff --git a/hledger/Hledger/Cli/Commands/Roi.hs b/hledger/Hledger/Cli/Commands/Roi.hs index ec7a239f0..ef083a92a 100644 --- a/hledger/Hledger/Cli/Commands/Roi.hs +++ b/hledger/Hledger/Cli/Commands/Roi.hs @@ -65,9 +65,15 @@ roi CliOpts{rawopts_=rawopts, reportspec_=rspec@ReportSpec{_rsReportOpts=ReportO -- lbl = lbl_ "roi" today = _rsDay rspec priceOracle = journalPriceOracle infer_prices_ j - styles = journalCommodityStyles j + styles = journalCommodityStylesWith HardRounding j mixedAmountValue periodlast date = - maybe id (mixedAmountApplyValuation priceOracle styles periodlast today date) value_ + -- These calculations can generate very precise decimals. To avoid showing too many digits: + -- If we have no style for the valuation commodity, generate one that will limit the precision ? + -- But it's not easy to find out the valuation commodity (or commodities) here if it's implicit, + -- as that information is buried in the price graph. + -- Instead, do what we don't like to do: hard code a max precision, overriding commodity styles. + mixedAmountSetPrecisionMax defaultMaxPrecision + . maybe id (mixedAmountApplyValuation priceOracle styles periodlast today date) value_ . maybe id (mixedAmountToCost styles) conversionop_ let diff --git a/hledger/test/roi.test b/hledger/test/roi.test index 3c78c93a2..49afdb787 100644 --- a/hledger/test/roi.test +++ b/hledger/test/roi.test @@ -257,7 +257,9 @@ $ hledger -f- roi -p 2019-11 --inv Investment --pnl PnL --cost --value=then,A -- >= 0 -# ** 11. Use "then" prices. 10000/76.20 = 131.23, 11000/73.88=148.89 +# ** 11. Use "then" prices. 10000/76.20 = 131.23, 11000/73.88=148.89. +# Also note that large decimals are limited to 8 digits if there's no +# standard display precision for them (P directives do not set display precision). < P 2020-12-01 $ 76.20 P 2021-01-01 $ 73.88 @@ -269,17 +271,20 @@ P 2021-01-01 $ 73.88 2021-01-02 get profit assets:investment =11000 income:investment + $ hledger -f - roi --inv assets:investment --pnl income:investment --value=then,'$' -+---++------------+------------++---------------+----------+-------------+-----++---------++------------+----------+ -| || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year | -+===++============+============++===============+==========+=============+=====++=========++============+==========+ -| 1 || 2020-12-02 | 2021-01-02 || 0 | $131 | $149 | $18 || 321.99% || 13.45% | 323.47% | -+---++------------+------------++---------------+----------+-------------+-----++---------++------------+----------+ ++---++------------+------------++---------------+---------------+---------------+--------------++---------++------------+----------+ +| || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year | ++===++============+============++===============+===============+===============+==============++=========++============+==========+ +| 1 || 2020-12-02 | 2021-01-02 || 0 | $131.23359580 | $148.89009204 | $17.65649624 || 321.99% || 13.45% | 323.47% | ++---++------------+------------++---------------+---------------+---------------+--------------++---------++------------+----------+ ->= 0 +>= -# ** 12. Use "end" prices. 10000/73.88=135.35 +# ** 12. Use "end" prices. 10000/73.88=135.35. +# And, large decimals can be rounded further with a commodity directive. < +commodity $1000.00 P 2020-12-01 $ 76.20 P 2021-01-01 $ 73.88 @@ -290,14 +295,15 @@ P 2021-01-01 $ 73.88 2021-01-02 get profit assets:investment =11000 income:investment -$ hledger -f - roi --inv assets:investment --pnl income:investment --value=end,'$' -+---++------------+------------++---------------+----------+-------------+-----++---------++------------+----------+ -| || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year | -+===++============+============++===============+==========+=============+=====++=========++============+==========+ -| 1 || 2020-12-02 | 2021-01-02 || 0 | $135 | $149 | $14 || 196.58% || 10.00% | 197.46% | -+---++------------+------------++---------------+----------+-------------+-----++---------++------------+----------+ ->= 0 +$ hledger -f - roi --inv assets:investment --pnl income:investment --value=end,'$' ++---++------------+------------++---------------+----------+-------------+--------++---------++------------+----------+ +| || Begin | End || Value (begin) | Cashflow | Value (end) | PnL || IRR || TWR/period | TWR/year | ++===++============+============++===============+==========+=============+========++=========++============+==========+ +| 1 || 2020-12-02 | 2021-01-02 || 0 | $135.35 | $148.89 | $13.54 || 196.58% || 10.00% | 197.46% | ++---++------------+------------++---------------+----------+-------------+--------++---------++------------+----------+ + +>= # ** 13. Several PnL transactions on a single date are aggregated together <