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

MersenneTwister: implement generation of jump-ahead polynomials #16906

Merged
merged 1 commit into from
Dec 18, 2017

Conversation

rfourquet
Copy link
Member

This completes PR #12498, which provided the jump-ahead functionality, by allowing to specify an arbitrary number of steps instead of the hard-coded 10^20. This code may very well be better-suited for a package, but it's so tiny (roughly the same size as the original code from #12498) that I thought I could push it here as well.

This is WIP as I'm considering evolving the randjump API, e.g. deprecating the method randjump(rng::MersenneTwister, jumps, jumppoly::AbstractString). Also I think I would like to have randjump(rng, steps) (and randjump!(rng, steps)) but this is incompatible with randjump(rng, jumps).

@rfourquet rfourquet force-pushed the rf/randjump-genpoly branch 2 times, most recently from 22f74b8 to 7cbeb70 Compare June 13, 2016 14:05
base/dSFMT.jl Outdated
end

"Cached jump polynomials for `MersenneTwister`."
# hack: contains Poly19937 at key -1 (as it can not be initialized at compile time)
Copy link
Contributor

Choose a reason for hiding this comment

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

need to double check this, but having the comment between the docstring and the const might mean the docstring is not associated with the const?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked and it works! but I'll move anyway the comment after the definition.

@wildart
Copy link
Member

wildart commented Jun 13, 2016

You can make jumps parameter in randjump to default to 1. And add steps as keyword argument so

randjump(rng, 10) # ten jumps with default steps
randjump(rng, steps=10) # one jump with 10 steps
randjump(rng, 10, steps=10) # ten jumps with 10 steps each

It's the same thing for randjump!.

base/random.jl Outdated
randjump(r::MersenneTwister, jumps, jumppoly::AbstractString) -> Vector{MersenneTwister}

Similar to `randjump(r, jumps, steps)` where the number of steps is determined
by a jump polynomial `jumppoly` in ``GF(2)[X]`` encoded as an hexadecimal string.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe explain in words what GF(2)[X] means?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@rfourquet rfourquet added the randomness Random number generation and the Random stdlib label Jun 15, 2016
@rfourquet rfourquet force-pushed the rf/randjump-genpoly branch 2 times, most recently from a573099 to 426ad10 Compare June 15, 2016 12:04
@rfourquet
Copy link
Member Author

I think I agree with your proposed API for randjum. I will remove the WIP, as I now think it's better to discuss an updated API in a new PR, which will depend on wheter this present PR gets merged or not.

@rfourquet rfourquet changed the title [WIP] MersenneTwister: implement generation of jump-ahead polynomials MersenneTwister: implement generation of jump-ahead polynomials Jun 15, 2016
@rfourquet
Copy link
Member Author

This is freshly rebased. Barring objections, I will just merge this: this adds only a little bit of code, and adds the missing half of the randjump functionality. And with Random moving to "stdlib", it makes even more sense, as opposed to putthing this into a separate package. Moreover, this will be useful as a complement of #25129.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants