From af5eaffa185ba87b5389e44592425dceec7bb30b Mon Sep 17 00:00:00 2001 From: Rafael Fourquet Date: Sun, 6 Jun 2021 14:53:53 +0200 Subject: [PATCH] simplify seeding for Xoshiro/TaskLocalRNG/GLOBAL_RNG 1. `seed!(rng::Xoshiro, seed::NTuple{4,UInt64})` and `seed!(rng::TaskLocalRNG, seed::NTuple{4,UInt64})` were doing almost the same thing; factor out what was identical; 2. `seed!(rng::Union{TaskLocalRNG, Xoshiro})` was calling the above methods passing a random tuple generated by `RandomDevice`: in this case, we don't really need to hash the seed, as it's presumably random enough; so use the same algorithm as in `Xoshiro()` constructor, and let `Xoshiro()` re-use this new implementation of `seed!`. --- stdlib/Random/src/RNGs.jl | 14 ++--- stdlib/Random/src/Xoshiro.jl | 96 +++++++++++++++++----------------- stdlib/Random/test/runtests.jl | 34 ++++++++++++ 3 files changed, 89 insertions(+), 55 deletions(-) diff --git a/stdlib/Random/src/RNGs.jl b/stdlib/Random/src/RNGs.jl index 45d0934dc4039..d3582b893d616 100644 --- a/stdlib/Random/src/RNGs.jl +++ b/stdlib/Random/src/RNGs.jl @@ -377,15 +377,15 @@ copy(::_GLOBAL_RNG) = copy(default_rng()) GLOBAL_SEED = 0 -seed!(::_GLOBAL_RNG, seed) = (global GLOBAL_SEED = seed; seed!(default_rng(), seed)) - -function seed!(rng::_GLOBAL_RNG) - seed!(rng, (rand(RandomDevice(), UInt64), rand(RandomDevice(), UInt64), - rand(RandomDevice(), UInt64), rand(RandomDevice(), UInt64))) +function seed!(::_GLOBAL_RNG, seed=rand(RandomDevice(), UInt64, 4)) + global GLOBAL_SEED = seed + seed!(default_rng(), seed) end -seed!() = seed!(GLOBAL_RNG) + seed!(rng::_GLOBAL_RNG, ::Nothing) = seed!(rng) # to resolve ambiguity -seed!(seed::Union{Integer,Vector{UInt32},Vector{UInt64},NTuple{4,UInt64}}) = seed!(GLOBAL_RNG, seed) + +seed!(seed::Union{Nothing,Integer,Vector{UInt32},Vector{UInt64},NTuple{4,UInt64}}=nothing) = + seed!(GLOBAL_RNG, seed) rng_native_52(::_GLOBAL_RNG) = rng_native_52(default_rng()) rand(::_GLOBAL_RNG, sp::SamplerBoolBitInteger) = rand(default_rng(), sp) diff --git a/stdlib/Random/src/Xoshiro.jl b/stdlib/Random/src/Xoshiro.jl index c1c37b3c1c134..8fd866732dbf9 100644 --- a/stdlib/Random/src/Xoshiro.jl +++ b/stdlib/Random/src/Xoshiro.jl @@ -25,23 +25,15 @@ mutable struct Xoshiro <: AbstractRNG s3::UInt64 Xoshiro(s0::Integer, s1::Integer, s2::Integer, s3::Integer) = new(s0, s1, s2, s3) - Xoshiro(seed) = seed!(new(), seed) + Xoshiro(seed=nothing) = seed!(new(), seed) end -Xoshiro(::Nothing) = Xoshiro() - -function Xoshiro() - parent = RandomDevice() - # Constants have nothing up their sleeve, see task.c - # 0x02011ce34bce797f == hash(UInt(1))|0x01 - # 0x5a94851fb48a6e05 == hash(UInt(2))|0x01 - # 0x3688cf5d48899fa7 == hash(UInt(3))|0x01 - # 0x867b4bb4c42e5661 == hash(UInt(4))|0x01 - - Xoshiro(0x02011ce34bce797f * rand(parent, UInt64), - 0x5a94851fb48a6e05 * rand(parent, UInt64), - 0x3688cf5d48899fa7 * rand(parent, UInt64), - 0x867b4bb4c42e5661 * rand(parent, UInt64)) +function setstate!(x::Xoshiro, s0::UInt64, s1::UInt64, s2::UInt64, s3::UInt64) + x.s0 = s0 + x.s1 = s1 + x.s2 = s2 + x.s3 = s3 + x end copy(rng::Xoshiro) = Xoshiro(rng.s0, rng.s1, rng.s2, rng.s3) @@ -57,18 +49,6 @@ end rng_native_52(::Xoshiro) = UInt64 -function seed!(rng::Xoshiro, seed::NTuple{4,UInt64}) - s = Base.hash_64_64(seed[1]) - rng.s0 = s - s += Base.hash_64_64(seed[2]) - rng.s1 = s - s += Base.hash_64_64(seed[3]) - rng.s2 = s - s += Base.hash_64_64(seed[4]) - rng.s3 = s - rng -end - @inline function rand(rng::Xoshiro, ::SamplerType{UInt64}) s0, s1, s2, s3 = rng.s0, rng.s1, rng.s2, rng.s3 tmp = s0 + s3 @@ -108,24 +88,13 @@ struct TaskLocalRNG <: AbstractRNG end TaskLocalRNG(::Nothing) = TaskLocalRNG() rng_native_52(::TaskLocalRNG) = UInt64 -function seed!(rng::TaskLocalRNG, seed::NTuple{4,UInt64}) - # TODO: Consider a less ad-hoc construction - # We can afford burning a handful of cycles here, and we don't want any - # surprises with respect to bad seeds / bad interactions. +function setstate!(x::TaskLocalRNG, s0::UInt64, s1::UInt64, s2::UInt64, s3::UInt64) t = current_task() - s = Base.hash_64_64(seed[1]) - t.rngState0 = s - s += Base.hash_64_64(seed[2]) - t.rngState1 = s - s += Base.hash_64_64(seed[3]) - t.rngState2 = s - s += Base.hash_64_64(seed[4]) - t.rngState3 = s - rand(rng, UInt64) - rand(rng, UInt64) - rand(rng, UInt64) - rand(rng, UInt64) - rng + t.rngState0 = s0 + t.rngState1 = s1 + t.rngState2 = s2 + t.rngState3 = s3 + x end @inline function rand(::TaskLocalRNG, ::SamplerType{UInt64}) @@ -145,6 +114,41 @@ end end # Shared implementation between Xoshiro and TaskLocalRNG -- seeding + +function seed!(x::Union{TaskLocalRNG,Xoshiro}) + # as we get good randomness from RandomDevice, we can skip hashing + parent = RandomDevice() + # Constants have nothing up their sleeve, see task.c + # 0x02011ce34bce797f == hash(UInt(1))|0x01 + # 0x5a94851fb48a6e05 == hash(UInt(2))|0x01 + # 0x3688cf5d48899fa7 == hash(UInt(3))|0x01 + # 0x867b4bb4c42e5661 == hash(UInt(4))|0x01 + setstate!(x, + 0x02011ce34bce797f * rand(parent, UInt64), + 0x5a94851fb48a6e05 * rand(parent, UInt64), + 0x3688cf5d48899fa7 * rand(parent, UInt64), + 0x867b4bb4c42e5661 * rand(parent, UInt64)) +end + +function seed!(rng::Union{TaskLocalRNG,Xoshiro}, seed::NTuple{4,UInt64}) + # TODO: Consider a less ad-hoc construction + # We can afford burning a handful of cycles here, and we don't want any + # surprises with respect to bad seeds / bad interactions. + + s0 = s = Base.hash_64_64(seed[1]) + s1 = s += Base.hash_64_64(seed[2]) + s2 = s += Base.hash_64_64(seed[3]) + s3 = s += Base.hash_64_64(seed[4]) + + setstate!(rng, s0, s1, s2, s3) + + rand(rng, UInt64) + rand(rng, UInt64) + rand(rng, UInt64) + rand(rng, UInt64) + rng +end + function seed!(rng::Union{TaskLocalRNG, Xoshiro}, seed::UInt128) seed0 = seed % UInt64 seed1 = (seed>>>64) % UInt64 @@ -152,10 +156,6 @@ function seed!(rng::Union{TaskLocalRNG, Xoshiro}, seed::UInt128) end seed!(rng::Union{TaskLocalRNG, Xoshiro}, seed::Integer) = seed!(rng, UInt128(seed)) -seed!(rng::Union{TaskLocalRNG, Xoshiro}) = - seed!(rng, (rand(RandomDevice(), UInt64), rand(RandomDevice(), UInt64), - rand(RandomDevice(), UInt64), rand(RandomDevice(), UInt64))) - function seed!(rng::Union{TaskLocalRNG, Xoshiro}, seed::AbstractVector{UInt64}) if length(seed) > 4 throw(ArgumentError("seed should have no more than 256 bits")) diff --git a/stdlib/Random/test/runtests.jl b/stdlib/Random/test/runtests.jl index 2c1230c0753d3..774b59664cfed 100644 --- a/stdlib/Random/test/runtests.jl +++ b/stdlib/Random/test/runtests.jl @@ -698,6 +698,40 @@ end end end +@testset "Random.seed!(seed) sets Random.GLOBAL_SEED" begin + seeds = Any[0, rand(UInt128), rand(UInt64, 4), Tuple(rand(UInt64, 4))] + + for seed=seeds + Random.seed!(seed) + @test Random.GLOBAL_SEED === seed + end + # two separate loops as otherwise we are no sure that the second call (with GLOBAL_RNG) + # actually sets GLOBAL_SEED + for seed=seeds + Random.seed!(Random.GLOBAL_RNG, seed) + @test Random.GLOBAL_SEED === seed + end + + Random.seed!(nothing) + seed1 = Random.GLOBAL_SEED + @test seed1 isa Vector{UInt64} # could change, but must not be nothing + + Random.seed!(Random.GLOBAL_RNG, nothing) + seed2 = Random.GLOBAL_SEED + @test seed2 isa Vector{UInt64} + @test seed2 != seed1 + + Random.seed!() + seed3 = Random.GLOBAL_SEED + @test seed3 isa Vector{UInt64} + @test seed3 != seed2 + + Random.seed!(Random.GLOBAL_RNG) + seed4 = Random.GLOBAL_SEED + @test seed4 isa Vector{UInt64} + @test seed4 != seed3 +end + struct RandomStruct23964 end @testset "error message when rand not defined for a type" begin @test_throws ArgumentError rand(nothing)