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

Enhancements to random number generators #4193

Open
farrellm opened this issue Nov 13, 2017 · 7 comments
Open

Enhancements to random number generators #4193

farrellm opened this issue Nov 13, 2017 · 7 comments

Comments

@farrellm
Copy link

farrellm commented Nov 13, 2017

I'm thinking of coding up a few enhancements to the existing library support for random number generators but looking for some feedback and advice on where to put it.

  1. Port System.Random from Haskell. This adds the RandomGen interface.
  2. Should System.Random go in contrib for now?
  3. Update Effect.Random to use any RandomGen as it's state
  4. Assuming 2+3, can the effects library depend on contrib? If not, how to structure this?
  5. Existing implementation of rndInt is not optimal, it uses the least significant bits, whereas Numerical Recipes in C states we should always use the most significant bits.

I have most of this coded already, and interestingly, I can avoid changing the existing Effect.Random interface at all by adding an implementation of RandomGen for Integer.

@farrellm
Copy link
Author

I should add, I could put System.Random in a third party library, but I was hoping to make Effect.Random depend on it, which would exclude third party libraries.

@farrellm
Copy link
Author

I suppose the other option is to put System.Random in the effects library. A bit odd, but maybe the simplest solution.

@farrellm
Copy link
Author

  1. Switch generated type from Integer to Int?

Seems like random number generation is one of the places we would prefer the performance of Int. Maybe the old RNG needed Integer for the calculation? But I think the return type should be Int.

@rbarreiro
Copy link
Collaborator

rbarreiro commented Nov 13, 2017 via email

@ahmadsalim
Copy link

@farrellm You are welcome to open a PR improving the random generation.
As @rbarreiro mentioned, it would be good if you added the changes to Control.ST in contrib.

@ahmadsalim
Copy link

@farrellm Additionally, please submit your contributions primarily to contrib, including System.Random.

@farrellm
Copy link
Author

Makes sense, I'll move my changes from Effect to Control.ST

A note on the tutorial, I saw the notice, but read it as only Effect.ST being deprecated. Probably worth adding a note explicitly stating the entire effects library is deprecated.

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

No branches or pull requests

3 participants