Skip to content
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

Lower memory use of instance Arbitrary Double/Float. #419

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ChickenProp
Copy link

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 #418.

    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.
Comment on lines 725 to 726
i <- chooseInt (0, n)
pure (fromRational (streamNth i rationalUniverse))
pure (fromRational (streamNth (min i 256) rationalUniverse))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates a strange distribution of min i 256. Better to do i <- chooseInt (0, min n 256).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure about that. I went with this on grounds "larger size means you still get bigger things here on average", but happy to change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with the old code is that larger size => n is almost always bigger than 256 => the result is almost always the same.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instance Arbitrary Double uses lots of memory with high sizes
2 participants