-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Random: support threads for GLOBAL_RNG access #32407
Conversation
From triage: make default global RNG per-thread for now. Document that explicit RNG objects are not thread-safe. |
General thoughts from triage:
|
Calling the function to get the global RNG |
Or maybe just |
Rebased to pick up #32604. I think we can merge this, and think about the naming convention for functions like this. I know people might start using it, but it's not exported and we'll have time to rename it if we want before 1.3. |
stdlib/Random/src/RNGs.jl
Outdated
#RNG = get(tls, :RNG, nothing) | ||
#RNG isa MersenneTwister && return RNG | ||
if length(THREAD_RNGs) < tid | ||
resize!(THREAD_RNGs, Threads.nthreads()) |
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.
I'm not sure whether this is thread safe.
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.
Yeah, I still need to push my local fork which fixes some stuff like this. Was just waiting on the outcome of the naming discussion.
The `Random.GLOBAL_RNG` is now a singleton placeholder object which implements the prior `Random` public API for MersenneTwister as a shim to support existing clients until Julia v2.0.
I agree with this suggestion, and had the same idea when reading this PR the first time. I find both |
@testset "GLOBAL_RNG" begin | ||
local GLOBAL_RNG = Random.GLOBAL_RNG | ||
local LOCAL_RNG = Random.get_local_rng() | ||
@test VERSION < v"2" # deprecate this in v2 |
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.
The _GLOBAL_RNG
thing is rather clever! Do I get it right that it's only to avoid breaking code and that it's intended to be removed in v2?
What is the reason that in this PR separate RNG stream are created (link to code) instead of using the "trick" advocated in the docs of using just one chain but sampling from different locations (implemented using the Wikipedia (fourth bullet) suggests that at least for MCMC the randjump strategy should be used. Or are the seeds of the separate streams chosen such that inter-stream numbers are non-correlated too? This line suggest that no such approach is taken. |
No description provided.