git.iko.soy/rowtype-yoga/purescript-fixed-precision
on main help sign in
Fix inaccuracy in `fromNumber`, fixes #17 (#18)

BigInt.fromNumber floors its argument; round the number first so that we don't end up with a BigInt which is 1 less than what we want.

commite0aac76668d1fcaa163e59770dee7183912aeb08 authorHarry Garrood <harry@garrood.me> dateMon 20 May 2019, 17:45 (7 years ago) parent7b8cbe3
2 files changed+33−2
src/Data/Fixed.purs+1−1 test/Main.purs+32−1
src/Data/Fixed.purs+1−1
@@ -190,7 +190,7 @@ fromNumber
190190 . KnownPrecision precision
191191 => Number
192192 -> Maybe (Fixed precision)
193−fromNumber n = map Fixed (BigInt.fromNumber (n * BigInt.toNumber (reflectPrecision (PProxy :: PProxy precision))))
193+fromNumber n = map Fixed (BigInt.fromNumber (Math.round (n * BigInt.toNumber (reflectPrecision (PProxy :: PProxy precision)))))
194194
195195-- | Convert a `Fixed` value to a `Number`.
196196-- |
test/Main.purs+32−1
@@ -2,10 +2,12 @@ module Test.Main where
22
33import Prelude
44
5−import Data.Fixed (class KnownPrecision, Fixed, PProxy, fromNumber, fromString, reifyPrecision, toString)
5+import Data.BigInt as BigInt
6+import Data.Fixed (class KnownPrecision, Fixed, PProxy, fromNumber, toNumber, fromString, reifyPrecision, reflectPrecision, toString)
67import Data.Maybe (Maybe(..), fromJust, isJust)
78import Effect (Effect)
89import Effect.Console (log)
10+import Math as Math
911import Partial.Unsafe (unsafePartial)
1012import Test.QuickCheck (arbitrary, quickCheck', (<?>))
1113import Test.QuickCheck.Gen (Gen, chooseInt, suchThat)
@@ -17,6 +19,14 @@ trying gen f = map (unsafePartial fromJust) (map f gen `suchThat` isJust)
1719genFixedP :: forall p. KnownPrecision p => PProxy p -> Gen (Fixed p)
1820genFixedP _ = map fromNumber arbitrary `trying` identity
1921
22+fromNumberWith
23+ :: forall precision
24+ . KnownPrecision precision
25+ => PProxy precision
26+ -> Number
27+ -> Maybe (Fixed precision)
28+fromNumberWith _ = fromNumber
29+
2030main :: Effect Unit
2131main = do
2232 log "Commutative monoid under addition:"
@@ -87,3 +97,24 @@ main = do
8797 unsafePartial fromJust $ reifyPrecision precision \p -> do
8898 a <- genFixedP p
8999 pure $ fromString (toString a) == Just a <?> show { precision, a }
100+
101+ log ""
102+ log "Conversions to/from Number"
103+ log ""
104+
105+ log "Roundtrip inaccuracy should be bounded by precision"
106+ quickCheck' 1000 \x -> do
107+ -- This is limited by the precision of Number
108+ precision <- chooseInt 0 10
109+ pure $ unsafePartial fromJust $ reifyPrecision precision \p -> do
110+ -- Note: we use 0.5 here because these numbers should differ by no more
111+ -- than half of the distance between two consecutive representable values
112+ -- of the type `Fixed precision`.
113+ let epsilon = 0.5 / BigInt.toNumber (reflectPrecision p)
114+ case toNumber <$> fromNumberWith p x of
115+ Just x' ->
116+ (Math.abs (x' - x) <= epsilon)
117+ <?> show { precision, epsilon, x, x' }
118+ Nothing ->
119+ false
120+ <?> ("failed to roundtrip via Number: " <> show { precision, x })