JuliaRandom / Random123.jl

Julia implementation of Random123.
http://juliarandom.github.io/RandomNumbers.jl/stable/man/random123/
Other
18 stars 5 forks source link

Philox generators don't increment second counter #5

Closed maleadt closed 3 years ago

maleadt commented 3 years ago

I'm trying to understand the implementation, so maybe I'm missing something, but it doesn't seem that this implementation correctly handles the second counter of, e.g., Philox2x? In the reference implementation, there is:

static unsigned long int NAME##_get(void *vstate){                      \
    NAME##_state *st = (NAME##_state *)vstate;                          \
    const int N=sizeof(st->ctr.v)/sizeof(st->ctr.v[0]);                 \
    if( st->elem == 0 ){                                                \
        ++st->ctr.v[0];                                                 \
        if( N>1 && st->ctr.v[0] == 0 ) ++st->ctr.v[1];                  \
        if( N>2 && st->ctr.v[1] == 0 ) ++st->ctr.v[2];                  \
        if( N>3 && st->ctr.v[2] == 0 ) ++st->ctr.v[3];                  \
        st->r = CBRNGNAME(st->ctr, st->key);                                 \
        st->elem = N;                                                   \
    }                                                                   \
    return 0xffffffffUL & st->r.v[--st->elem];                          \
}       

Furthermore, when the first counter runs out, the code crashes:

julia> using Random123

julia> rng = Philox2x(UInt32)
Philox2x{UInt32, 10}(0xfce15e42, 0x8ffc34da, 0x88bd3fad, 0x00000000, 0x00000000, 0)

julia> rng.ctr1 = typemax(UInt32)
0xffffffff

julia> rand(rng)
0.9878138457424939

julia> rand(rng)
ERROR: InexactError: trunc(UInt32, 4294967296)
Stacktrace:
 [1] throw_inexacterror(f::Symbol, #unused#::Type{UInt32}, val::Int64)
   @ Core ./boot.jl:602
 [2] checked_trunc_uint
   @ ./boot.jl:632 [inlined]
 [3] toUInt32
   @ ./boot.jl:716 [inlined]
 [4] UInt32
   @ ./boot.jl:756 [inlined]
 [5] convert
   @ ./number.jl:7 [inlined]
 [6] setproperty!
   @ ./Base.jl:34 [inlined]
 [7] rand
   @ ~/Julia/depot/packages/Random123/cb3eU/src/common.jl:49 [inlined]
 [8] rand (repeats 2 times)
   @ ~/Julia/depot/packages/RandomNumbers/m57pA/src/common.jl:25 [inlined]
 [9] top-level scope
   @ REPL[4]:1
sunoru commented 3 years ago

You are right. I don't remember why only the first counter was used. I'm gonna fix it in an update.

sunoru commented 3 years ago

Fixed