re-enable the 0-arg MersenneTwister() constructor #21909
Conversation
|
Rebased. I will merge in a few days if no objection. |
rfourquet
changed the title
rfourquet
force-pushed
the
rf/MT-ctor
branch
from
e689b8a
to
b1ce1e8
Compare
The arguments to `srand(rng, ...)` and `typeof(rng)(...)` should mirror each-other, in particular `srand(MersenneTwister(0))` and `MersenneTwister()` should produce an equivalent object. Also, `srand(::RandomDevice)` has been added for consistency, which could be useful in generic code.
rfourquet
force-pushed
the
rf/MT-ctor
branch
from
b1ce1e8
to
81b839d
Compare
| """ | ||
| RandomDevice | ||
|
|
||
| RandomDevice(::Void) = RandomDevice() |
There was a problem hiding this comment.
It's not. For MersenneTwister, I introduced it for convenience (but still not necessary), and I thought it could be a nice API (not yet documented): when you want to initialize your RNG, you can default your seed to nothing to have the behavior of non-reproducibility. WIthout nothing, you have to handle the 2 cases:
seed = Nullable{Int}()
...
rng = isnull(seed) ? MersenneTwister() : MersenneTwister(seed)
# vs:
rng = MersenneTwister(get(seed, nothing))
There was a problem hiding this comment.
couldn't you splat an empty tuple? we don't often use nothing as an input flag, other than unset keywords
There was a problem hiding this comment.
couldn't you splat an empty tuple? we don't often use nothing as an input flag, other than unset keywords
I guess splatting a tuple would work... but it seams more natural to me to use nothing, and your mention of keyword is helpful: it's easy to have an API like do_computation(...; seed=nothing) and feed the RNG directly with seed without having to splat a tuple. I remember few times having found inconvenient to not be able to give a default value to a seed which could mean "seed randomly". A couple of relevant (kind of) examples which receive a seed as parameter which could benefit from allowing nothing: https://github.com/denizyuret/Knet.jl/pull/105/files and https://github.com/JuliaGraphs/LightGraphs.jl/pull/579/files
Cf. #16984 for context, which deprecated
MersenneTwister() = MersenneTwister(0).The next step here is to re-enable this constructor to be equivalent to
srand(MersenneTwister(0)).I think that it's good for predictability that for an RNG type
T,T([seed])produces an RNG object equivalent tosrand(T(), [seed])).Accordingly,
srand(RandomDevice())is also enabled (as a no-op).I put WIP as I'm not satisfied with my wording in the docstrings.