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

RFC: Adds random number generation for BigInts and BigFloats #4845

Merged
merged 9 commits into from
Dec 25, 2013

Conversation

andrioni
Copy link
Member

This pull request extends the work done by @jiahao in #3077, adding a default RNG for BigInts and BigFloats (since they share the gmp_randstate interface) and BigFloat support (including for randn).

I'll update it later with the documentation as soon as we settle on the interface.

@johnmyleswhite
Copy link
Member

Very nice. Don't let this question slow you down, but is there any way we could get GMP to sync up with the Julia RNG state?

@StefanKarpinski
Copy link
Member

This is a great pull request. I'm not sure how one would sync up the RNGs since it seems like BigRNG probably needs a lot more state than dSFMT.

@johnmyleswhite
Copy link
Member

That's probably true: hadn't thought about that at all.

@andrioni
Copy link
Member Author

Well, technically we could make GMP use Julia's dSFMT instead of its own internal Mersenne Twister, but it'll require large amounts of glue code to make it work.

@johnmyleswhite
Copy link
Member

Probably not worth it.

@jiahao
Copy link
Member

jiahao commented Nov 18, 2013

@andrioni thanks for picking this up again. I had completely forgotten about this.

@JeffBezanson
Copy link
Member

Do we want to be able to reproduce the same sequence of all random numbers (big or small) from a single srand call?

@ViralBShah
Copy link
Member

It would be nice if we could, but may not be easy. What happens when we mix random numbers from BigRNG and existing RNGs?

@andrioni
Copy link
Member Author

Currently, both RNGs are completely independent. You have to reseed them separately if you want to, and they don't affect each other's state.

@stevengj
Copy link
Member

I would tend to prefer that srand seeded them both, by default.

@StefanKarpinski
Copy link
Member

That should be easy enough to do.

@ViralBShah
Copy link
Member

I would prefer seeding both with srand too. It would be nice to have some tests and docs as well before we merge.

OS X doesn't have `sha1sum` available, so the fallback used to get
the initial seed whenever /dev/urandom isn't available doesn't work.
@andrioni
Copy link
Member Author

I've just changed the srand behavior, added some tests and documentation.

(I have no idea whatsoever why travis failed, especially since it worked here with the same commit)

@ViralBShah
Copy link
Member

BTW, this should also get a mention in NEWS.md as we have started it for 0.3. Is this good to merge otherwise?

@andrioni
Copy link
Member Author

I think everything I need/wanted to do is done, so barring other suggestions/objections, it should be good to merge.

andrioni added a commit to andrioni/MPFI.jl that referenced this pull request Nov 22, 2013
It requires JuliaLang/julia#4845 to be merged, since it uses BigRNG.
andrioni added a commit to andrioni/MPFI.jl that referenced this pull request Nov 22, 2013
Now a macro is used to check if BigRNG is available, this means
the same codebase is compatible with both julia post-
JuliaLang/julia#4845 and julia 0.2.
@ViralBShah
Copy link
Member

The clang build prints a ton of garbage and dies. Perhaps worth investigating before merging.

@ViralBShah
Copy link
Member

Bump. @andrioni Can you please rebase and check if the tests are passing? This would be good to have.

@ViralBShah ViralBShah merged commit 4bf4829 into JuliaLang:master Dec 25, 2013
@ViralBShah
Copy link
Member

I was able to merge and get this running with all the tests passing.

convert(BigInt, first(r) + randu(ulen))
end

function rand!(r::Range1{BigInt}, A::Array{BigInt})
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant with definitions in random.jl

@stevengj
Copy link
Member

Since this patch was merged, why am I getting the error:

julia> rand(BigFloat)
ERROR: no random number generator for type BigFloat; try a more specific type
 in error at error.jl:21
 in rand at random.jl:138

?

@andrioni
Copy link
Member Author

It was rolled back after some time, since it was bugging out on some configurations (including the Travis one, IIRC).

@mschauer
Copy link
Contributor

@andrioni Do you recall what exacly went wrong?

@andrioni
Copy link
Member Author

@mschauer not off the top of my head, but I could take a look.

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.

8 participants