-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instance Arbitrary Double
uses lots of memory with high sizes
#418
Comments
I can only say that That said, the
feels the best solution. I.e. fix the particular instance in ad-hoc way. It's ad-hoc instance ( |
ghci> flip traverse [1..100] $ \_ -> length . show <$> generate (scale (const 100000000) $ (arbitrary :: Gen Double)) on master, this is slow and memory hungry. (I couldn't figure out how to pass RTS options to ghci using `cabal repl`, but I've seen it take over 5 minutes and 10 GB.) With this commit it's fast. Closes nick8325#418.
Nod. I've opened #419 for this. |
My code has been doing
scale (^ 8)
to generateDouble
s in between ±10^16 instead of in between ±100. But at that size, memory usage is very high since QuickCheck-2.14.3.(But I've also seen the heap overflow with
-M4G
.)I guess this is due to
smallDenominators
introduced in #297, which picks a number up tosize
and then uses O(n) time and memory to index into a tree of rationals.Some possibilities that come to mind:
Arbitrary
instances might not expect sizes that large, and users need to use other generators if they want that kind of thing. I don't think it would cause me big problems, but I don't love the idea. ("I want bigger numbers -> I'll use a bigger size" is pretty natural, which is how I came across this problem; and if the numbers are embedded in some other data structure, specifying another generator might be awkward.)smallDenominators
, limit the size to some relatively small max.I guess this would make it O(log n) time and memory (effectively O(1) because size isI expect this would be a lot faster and more memory efficient for largeInt
notInteger
).n
. If it's too slow for smalln
, then "build the first _ tree values, calculate directly from then on" could work.The text was updated successfully, but these errors were encountered: