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

Implement Uniform and UniformRange #14

Merged

Conversation

lehins
Copy link
Collaborator

@lehins lehins commented Feb 29, 2020

No description provided.

@lehins
Copy link
Collaborator Author

lehins commented Feb 29, 2020

This PR implements suggestion from here: #8 (comment)

Copy link

@Shimuuar Shimuuar left a comment

Choose a reason for hiding this comment

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

I like changes but didn't pay much attention to changes to Random


class UniformRange a where
uniformR :: MonadRandom g m => (a, a) -> g -> m a
Copy link

Choose a reason for hiding this comment

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

Here we have question of semantics. What should uniformR (a,b) do for a > b? random's position is that it's unspecified. mwc-random made choice uniformR (a,b) = uniformR (b,a)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that is a fine choice. It is definitely better than throwing an error. We could of course return a Maybe, but for consistency sake with other libraries and current randomR I think: uniformR (a,b) = uniformR (b,a) is the right way to go.

@lehins lehins mentioned this pull request Mar 1, 2020
@lehins
Copy link
Collaborator Author

lehins commented Mar 1, 2020

I am glad you like that changes, I am onboard with this as well. Merging, if any changes are necessary we can handle em in the bigger PR. It sound like a problem in #8 is solved too, so we can probably close it as well.

@lehins lehins merged commit 656ef17 into interface-to-performance Mar 1, 2020
@lehins lehins mentioned this pull request Mar 3, 2020
@curiousleo curiousleo deleted the interface-to-performance-uniform-range branch March 17, 2020 09:56
curiousleo pushed a commit that referenced this pull request May 19, 2020
…uniform-range

Implement Uniform and UniformRange
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.

3 participants