JuliaRandom / RandomNumbers.jl

Random Number Generators for the Julia Language.
https://juliarandom.github.io/RandomNumbers.jl/stable
Other
97 stars 23 forks source link

randfloat() for Float32/64 plus tests #73

Closed milankl closed 3 years ago

milankl commented 3 years ago

Sorry, how do you use Random.default_rng()? It's not <:AbstractRNG which I currently require in randfloat(rng::AbstractRNG... that's why it fails

milankl commented 3 years ago

I believe the failure in 1.5 will be fixed with https://github.com/sunoru/RandomNumbers.jl/pull/70/commits/235e93195f492944936ee5e6bdfcb9db342b941a, but Random.GLOBAL_RNG wasn't a <:AbstractRNG in 1.0? Sorry, I think I need your input here.

sunoru commented 3 years ago

Ah, yes, there's a subtle thing. You need to use Random.AbstractRNG here since I redefined an AbstractRNG in this package.

And you are right I need to merge the other PR first... I don't know why I missed that for so long :(

milankl commented 3 years ago

Sorry, that's what I currently have no?

import Random: AbstractRNG, GLOBAL_RNG
function randfloat(rng::AbstractRNG,::Type{Float32})
...
randfloat(::Type{T}=Float64) where T = randfloat(GLOBAL_RNG,T)
sunoru commented 3 years ago

I think it should be more explicit like

function randfloat(rng::Random.AbstractRNG,::Type{Float32})
sunoru commented 3 years ago

There's also a warning in the testing log showing the import is ignored: WARNING: import of Random.AbstractRNG into RandomNumbers conflicts with an existing identifier; ignored.

milankl commented 3 years ago

Okay sorry, I missed that! Let me remove that line then.

milankl commented 3 years ago

Finally 🎉

sunoru commented 3 years ago

Nice 🎉