-
Notifications
You must be signed in to change notification settings - Fork 50
random.monad.rkt: A first implementation of a PRNG Monad for Hackett #45
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
I just added one file, Anyway, I'm sure there are better people to implement this. I started on Racket only about 24 hours ago! So any feedback, postive or negative, is appreciated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is neat. I don’t have time to review this comprehensively right this moment, but I’ve left some comments on a few things that stood out. The most glaring issue, however, is that I don’t think it’s the right API.
You maintain purity by setting the seed to 1
on each pseudorandom computation, but this is awkward, since it means users must manually seed the generator. Furthermore, the entire computation can only be run in a monad, but the monad is semantically just a state monad that threads around a generator state.
I think you will achieve better luck if you represent a PRNG as a pure, opaque value that is internally backed by the result of pseudo-random-generator->vector
. That way, you can expose low-level, non-monadic, pure functions that operate directly on the generator and produce a new generator, which is a nicer core model. Building a monadic interface on that then becomes a derived concept. (Of course, Racket’s PRNG is not splittable, which means the PRNG implementation might not be the right one long-term for pure programs, anyway, but you probably don’t want to implement an entirely new PRNG just for Hackett, especially with its performance currently being rather poor.)
Additionally, your style is pretty all over the place—consider reading through the Racket Style Guide and cleaning things up a bit. To summarize:
-
Racket is not C, and close parentheses do not belong on their own line. Put them on the same line as the preceding expression. Always.
-
Indentation is based on expression structure. I recommend using an editor that can apply the proper indentation automatically, such as DrRacket or racket-mode for emacs.
-
Don’t include spaces in the middle of expressions for no reason. I’m not really sure what happened to end up with something like
( λ [f (prng mx)] ....)
.
(((hackett:random-world ; the three lambdas on the next three lines define the three functions needed for the interface to a generator | ||
(λ- (seed) (let ([old-rng (current-pseudo-random-generator) ]) | ||
(begin | ||
(current-pseudo-random-generator my-rng) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use parameterize
here instead of mutating the parameter.
) | ||
(((hackett:random-world ; the three lambdas on the next three lines define the three functions needed for the interface to a generator | ||
(λ- (seed) (let ([old-rng (current-pseudo-random-generator) ]) | ||
(begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This begin
is unnecessary (let
allows multiple subforms).
Thanks for the quick feedback. Comprehensive and very useful too, for this project and for my Racket generally! The only hesitation I have around the idea to back it with "... internally backed by the result of pseudo-random-generator->vector" is performance. This would rebuild a generator every time we want to draw a single random number. My assumption was that this would be slow. But maybe that assumption is incorrect, and even if it is correct maybe Hackett is going to be the bottleneck! Anyway, I can do some simple benchmarking. But yes, I can apply the changes you discussed. But I'm not sure when I'll have time. I enjoy this, but I should do other things too :-) |
@aaronmcdaid do you mind if I rewrite this? |
Go for it, @iitalics ! Any help/feedback appreciated I've just done some benchmarking which basically confirms that there is negligible cost from reseeding the generator (or, more precisely, (de-)serializing to/from the If I have time in the coming days, I'll probably keep playing with this. But I don't expect to be able to come up with anything great, so please don't hesitate to work on this! |
Cool -- I have a bit of code written already using ->vector etc., so I'm glad that there are no performance problems. I was planning on making a RandomT monad that is independent of the generator, e.g. (data (RandomT m a) (random-t (forall [g] {g -> (m (Tuple a g))}))) Unfortunately I ran into the typechecker problem I created an issue about. |
Should I make it (RandomT g m a) ? Or just make the generator type with state-monad-like functions and no monad instance? |
I must admit I can't answer those questions, @iitalics . I do understand - just about - what the questions mean, but I don't have the experience with these kinds of language to have any thoughts on the best interface I've put my code into a separate, much smaller, repo that is just this single file and a test program. Perhaps it would be easier for us to work in that? https://github.com/aaronmcdaid/random.hackett Or just ignore that if you like :-) PS: my own personal interest is to something deterministic and "splittable" and I hope to build some sort of MCMC library (Markov Chain Monte Carlo) on top of it. So I don't really need anything too flexible at the moment. |
This is a monad for accessing the random number generation facilities in Racket, as discussed on slack last night.
The file itself (
random.monad.rkt
) has a detailed description of what I did. This is "deterministic" currently, in the sense that each new generator created here is seeded with1
by default.I guess it might be useful to add something of type
(IO (PRNG a))
to access non-deterministic seeding if users want that