search

cartazio / old-random

Random number library
BSD 2-Clause "Simplified" License
3 stars 1 forks source link

Add PCG internal module #2

Closed cchalmers closed 7 years ago

cchalmers commented 7 years ago

Here's my PCG code as promised (#1) . I've also included the bounded random number code I've used in pcg-random.

The only thing I'm not sure about is my split. It passed big-crunch and die-harder but there's probably a better way to do it. Maybe we should ask the pcg author?

cartazio commented 7 years ago

Great!
If I don't get to code review in the day two days, please poke me via email or irc to get through it.

I've done a first pass read through and it looks good. But need to digest and compare with a few other things.

cartazio commented 7 years ago

One possibly silly question: why is there two exposed modules lines?

cchalmers commented 7 years ago

Ha, I just copied the line without thinking. Fixed it now, also took out the rest of the cabal comments.

cartazio commented 7 years ago

I generally like leaving the other comments in, but I won't object too strongly to the removal ... even though my own habits are to always leave them in the cabal file.

Could you maybe revert that part? 😇😊 On Sun, Feb 19, 2017 at 3:39 AM Chris notifications@github.com wrote:

Ha, I just copied the line without thinking. Fixed it now, also took out the rest of the cabal comments.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/cartazio/random/pull/2#issuecomment-280904642, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQwuHuyTCXGj_2HzETX6UXsUg23Zgmks5rd_-8gaJpZM4MFSGE .

cchalmers commented 7 years ago

Sure, I've revered it.

cchalmers commented 7 years ago

@cartazio Hey, this is me reminding you to look at this.

cartazio commented 7 years ago

Woot. Thanks. I'll poke at it this week On Mon, Mar 6, 2017 at 9:13 AM Chris notifications@github.com wrote:

@cartazio https://github.com/cartazio Hey, this is me reminding you to look at this.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/cartazio/random/pull/2#issuecomment-284406622, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQwgE5RohsMS2uktEiKEOn3CHBrS2Pks5rjBSYgaJpZM4MFSGE .

cartazio commented 7 years ago

@cchalmers i may or may not take an axe to the code to to clean up some style preferences (eg i prefer do notation for non recursive "imperative" code), but looks lovely and many thanks! :)