Repository: JuliaAI/MLJBase.jl (julia)

Issue:
Cannot pass arbitrary `rng`  to `make_blobs`
Although not in the doc string, you can pass `rng` to `make_blobs` (in addition to `shuffle`). This was presumable added for consistency of API elsewhere (see below). However, it does not work:

```julia
using MLJBase
using StableRNGs

julia> X, y = make_blobs(3, rng=rng)
ERROR: MethodError: no method matching seed!(::StableRNGs.LehmerRNG)
Closest candidates are:
  seed!(::AbstractRNG, ::Nothing) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Random/src/Random.jl:426
  seed!(::StableRNGs.LehmerRNG, ::Integer) at /Users/anthony/.julia/packages/StableRNGs/uwZ1I/src/StableRNGs.jl:39                   
  seed!() at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Random/src/RNGs.jl:362
  ...
Stacktrace:
 [1] shuffle_rows(X::Matrix{Float64}, Y::Vector{Int64}; rng::StableRNGs.LehmerRNG)
   @ MLJBase ~/Dropbox/Julia7/MLJ/MLJBase/src/utilities.jl:133
 [2] finalize_Xy(X::Matrix{Float64}, y::Vector{Int64}, shuffle::Bool, as_table::Bool, eltype
::Type, rng::StableRNGs.LehmerRNG; clf::Bool)                                              
   @ MLJBase ~/Dropbox/Julia7/MLJ/MLJBase/src/data/datasets_synthetic.jl:24
 [3] finalize_Xy(X::Matrix{Float64}, y::Vector{Int64}, shuffle::Bool, as_table::Bool, eltype
::Type, rng::StableRNGs.LehmerRNG)                                                         
   @ MLJBase ~/Dropbox/Julia7/MLJ/MLJBase/src/data/datasets_synthetic.jl:23
 [4] make_blobs(n::Int64, p::Int64; shuffle::Bool, centers::Int64, cluster_std::Float64, center_box::Pair{Float64, Float64}, as_table::Bool, eltype::Type{Float64}, rng::StableRNGs.LehmerRNG)                                                                       
   @ MLJBase ~/Dropbox/Julia7/MLJ/MLJBase/src/data/datasets_synthetic.jl:152
 [5] top-level scope
   @ REPL[17]:1
```

This is probably an issue with the other synthetic data generators. 

Elsewhere randomness is handled with user being able to specify kwargs `rng` and/or `shuffle` with defaults `nothing` and `false`. The logic is summarized in the `Holdout` doc-string, repeated below:

**edit** To this end one can use the helper [MLJBase.shuffle_and_rng](https://github.com/alan-turing-institute/MLJBase.jl/blob/fb183dc07d1b1c10bfc3aef23f7b61eae53abfc7/src/resampling.jl#L21)

---

Pre-shuffling of `rows` is controlled by `rng` and `shuffle`. If `rng`
is an integer, then the `Holdout` keyword constructor resets it to
`MersenneTwister(rng)`. Otherwise some `AbstractRNG` object is
expected.

If `rng` is left unspecified, `rng` is reset to `Random.GLOBAL_RNG`,
in which case rows are only pre-shuffled if `shuffle=true` is
specified.

cc @olivierlabayle

@giordano Can I interest you in picking this up?

Pull request description:
fixes #567 and does a little code cleanup.

---> on the `make_blobs` branch
```julia
julia> using MLJBase

julia> using StableRNGs

julia> rng = StableRNG(123)
StableRNGs.LehmerRNG(state=0x000000000000000000000000000000f7)

julia> X, y = make_blobs(3, rng=rng)
(Tables.MatrixTable{Matrix{Float64}} with 3 rows, 2 columns, and schema:
 :x1  Float64
 :x2  Float64, CategoricalArrays.CategoricalValue{Int64, UInt32}[1, 3, 2])
```

cc @ablaom , cc @giordano, cc @olivierlabayle

Patch:
diff --git a/src/data/datasets_synthetic.jl b/src/data/datasets_synthetic.jl
index f0425b34..942eea3f 100644
--- a/src/data/datasets_synthetic.jl
+++ b/src/data/datasets_synthetic.jl
@@ -3,7 +3,7 @@ const EXTRA_KW_MAKE = """
 * `eltype=Float64`: machine type of points (any subtype of
    `AbstractFloat`).
 
-* `rng=nothing`: any `AbstractRNG` object, or integer to seed a
+* `rng=Random.GLOBAL_RNG`: any `AbstractRNG` object, or integer to seed a
   `MersenneTwister` (for reproducibility).
 
 * `as_table=true`: whether to return the points as a table (true)
@@ -82,7 +82,7 @@ function make_blobs(n::Integer=100,
                     center_box::Pair{<:Real,<:Real}=(-10.0 => 10.0),
                     as_table::Bool=true,
                     eltype::Type{<:AbstractFloat}=Float64,
-                    rng=nothing)
+                    rng=Random.GLOBAL_RNG)
 
     # check arguments make sense
     if n < 1 || p < 1
@@ -94,13 +94,7 @@ function make_blobs(n::Integer=100,
             "Domain for the centers improperly defined expected a pair " *
             "`a => b` with `a < b`."))
     end
-
-    if rng === nothing
-        rng = Random.GLOBAL_RNG
-    elseif rng isa Integer
-        rng = Random.MersenneTwister(rng)
-    end
-
+    rng = init_rng(rng)
     if centers isa Matrix
         if size(centers, 2) != p
             throw(ArgumentError(
@@ -129,16 +123,16 @@ function make_blobs(n::Integer=100,
     end
 
     # split points equally among centers
-    ni, r    = divrem(n, n_centers)
-    ns       = fill(ni, n_centers)
+    ni, r = divrem(n, n_centers)
+    ns = fill(ni, n_centers)
     ns[end] += r
 
     # vector of memberships
     y = vcat((fill(i, ni) for (i, ni) in enumerate(ns))...)
 
     # Pre-generate random points then modify for each center
-    X    = randn(rng, n, p)
-    nss  = [1, cumsum(ns)...]
+    X = randn(rng, n, p)
+    nss = [1, cumsum(ns)...]
     # ranges of rows for each center
     rows = [nss[i]:nss[i+1] for i in 1:n_centers]
     @inbounds for c in 1:n_centers
@@ -189,7 +183,7 @@ function make_circles(n::Integer=100;
                       factor::Real=0.8,
                       as_table::Bool=true,
                       eltype::Type{<:AbstractFloat}=Float64,
-                      rng=nothing)
+                      rng=Random.GLOBAL_RNG)
 
     # check arguments make sense
     if n < 1
@@ -204,13 +198,8 @@ function make_circles(n::Integer=100;
         throw(ArgumentError(
             "Factor argument must be strictly between 0 and 1."))
     end
-
-        if rng === nothing
-                rng = Random.GLOBAL_RNG
-        elseif rng isa Integer
-                rng = Random.MersenneTwister(rng)
-        end
-
+    
+    rng = init_rng(rng)
     # Generate points on a 2D circle
     θs = runif_ab(rng, n, 1, 0, 2pi)
 
@@ -271,7 +260,7 @@ function make_moons(n::Int=150;
                     yshift::Real=0.3,
                     as_table::Bool=true,
                     eltype::Type{<:AbstractFloat}=Float64,
-                    rng=nothing)
+                    rng=Random.GLOBAL_RNG)
 
     # check arguments make sense
     if n < 1
@@ -283,12 +272,7 @@ function make_moons(n::Int=150;
             "Noise argument cannot be negative."))
     end
 
-        if rng === nothing
-                rng = Random.GLOBAL_RNG
-        elseif rng isa Integer
-                rng = Random.MersenneTwister(rng)
-        end
-
+    rng = init_rng(rng)
     n1 = div(n, 2)
     n2 = n - n1
 
@@ -407,7 +391,7 @@ function make_regression(n::Int=100,
                          binary::Bool=false,
                          as_table::Bool=true,
                          eltype::Type{<:AbstractFloat}=Float64,
-                         rng=nothing)
+                         rng=Random.GLOBAL_RNG)
 
     # check arguments make sense
     if n < 1 || p < 1
@@ -427,12 +411,7 @@ function make_regression(n::Int=100,
             "Outliers argument must be in [0, 1]."))
     end
 
-    if rng === nothing
-        rng = Random.GLOBAL_RNG
-    elseif rng isa Integer
-        rng = Random.MersenneTwister(rng)
-    end
-
+    rng = init_rng(rng)
     X = augment_X(randn(rng, n, p), intercept)
     θ = randn(rng, p + Int(intercept))
     sparse > 0 && sparsify!(rng, θ, sparse)
diff --git a/src/utilities.jl b/src/utilities.jl
index e2fa5c67..623dcb4b 100644
--- a/src/utilities.jl
+++ b/src/utilities.jl
@@ -123,19 +123,41 @@ function _permute_rows(obj::AbstractVecOrMat, perm::Vector{Int})
 end
 
 """
-shuffle_rows(X, Y, ...; rng=)
+shuffle_rows(X::AbstractVecOrMat, Y::AbstractVecOrMat; rng::AbstractRNG = Random.GLOBAL_RNG)
 
-Return a shuffled view of a vector or  matrix `X` (or set of such) using a
-random permutation (which can be seeded specifying `rng`).
+Return shuffled vectors or matrices using a random permutation of `X` and `Y`. An optional random 
+number generator can be specified using the `rng` argument.
 """
-function shuffle_rows(X::AbstractVecOrMat, Y::AbstractVecOrMat; rng=nothing)
+function shuffle_rows(
+    X::AbstractVecOrMat, Y::AbstractVecOrMat;
+    rng::AbstractRNG=Random.GLOBAL_RNG
+)
     check_dimensions(X, Y)
-    rng === nothing || Random.seed!(rng)
-    perm = randperm(size(X, 1))
+    perm_length = size(X, 1)
+    perm = randperm(rng, perm_length)
     return _permute_rows(X, perm), _permute_rows(Y, perm)
 end
 
-
+"""
+init_rng(rng)
+
+Create an `AbstractRNG` from `rng`. If `rng` is a non-negative `Integer`, it returns a
+`MersenneTwister` random number generator seeded with `rng`; If `rng` is 
+an `AbstractRNG` object it returns `rng`, otherwise it throws an error.
+""" 
+function init_rng(rng)
+    if (rng isa Integer && rng > 0)
+        return Random.MersenneTwister(rng)
+    elseif !(rng isa AbstractRNG)
+        throw(
+            ArgumentError(
+                "`rng` must either be a non-negative `Integer`, "*
+                "or an `AbstractRNG` object."
+            )
+        )
+    end
+    return rng
+end
 ## FOR PRETTY PRINTING
 
 # of coloumns:


Test patch:
diff --git a/test/hyperparam/one_dimensional_range_methods.jl b/test/hyperparam/one_dimensional_range_methods.jl
index 04b8c4fb..4b3068e4 100644
--- a/test/hyperparam/one_dimensional_range_methods.jl
+++ b/test/hyperparam/one_dimensional_range_methods.jl
@@ -41,6 +41,14 @@ p4 = range(dummy_model, :K, lower=1, upper=3, scale=x->2x)
 
 [p4, p4]
 
+# Starting from julia v"1.7.0-DEV.1233", the default RNG has changed
+# create a function giving julia version dependent default RNG with seed.
+@static if VERSION >= v"1.7.0-DEV.1230"
+    _default_rng(seed) = (rng = TaskLocalRNG(); Random.seed!(rng, seed))
+else
+    _default_rng(seed) = MersenneTwister(seed)
+end
+
 @testset "scale transformations" begin
     @test transform(MLJBase.Scale, scale(:log), ℯ) == 1
     @test inverse_transform(MLJBase.Scale, scale(:log), 1) == float(ℯ)
@@ -179,7 +187,7 @@ end
         @test all(x >= 0.2 for x in v)
         @test abs(minimum(v)/0.2 - 1) <= 0.01
 
-        rng = Random.MersenneTwister(1);
+        rng = _default_rng(1);
         @test rand(rng, s, 1000) == v
 
         q = quantile(v, 0.0:0.1:1.0)
@@ -190,23 +198,22 @@ end
     end
 
     @testset "sampler using callable scale" begin
-
         r = range(Int, :dummy, lower=1, upper=2, scale=x->10^x)
         s = sampler(r, Dist.Uniform)
         Random.seed!(123)
-        v = rand(s, 100)
+        v = rand(s, 10000)
         @test issubset(v, 10:100)
-        rng = MersenneTwister(123)
-        @test rand(rng, s, 100) == v
+        rng = _default_rng(123)
+        @test rand(rng, s, 10000) == v
 
         r = range(Float64, :dummy, lower=1, upper=2, scale=x->10^x)
         s = sampler(r, Dist.Uniform)
         Random.seed!(1)
-        v = rand(s, 1000)
+        v = rand(s, 10000)
         @test abs(minimum(v) - 10) < 0.02
         @test abs(maximum(v) - 100) < 0.02
-        rng = MersenneTwister(1)
-        @test rand(rng, s, 1000) == v
+        rng = _default_rng(1)
+        @test rand(rng, s, 10000) == v
 
     end
 
diff --git a/test/utilities.jl b/test/utilities.jl
index 2fb6f865..574ab42d 100644
--- a/test/utilities.jl
+++ b/test/utilities.jl
@@ -62,18 +62,27 @@ end
     @test MLJBase.check_dimensions(x, y) === nothing
     @test MLJBase.check_dimensions(z, x) === nothing
     @test MLJBase.check_dimensions(y, z) === nothing
-    @test_throws DimensionMismatch MLJBase.check_dimensions(x, randn(4))
+    @test_throws DimensionMismatch MLJBase.check_dimensions(x, randn(rng, 4))
 
     x = 1:5 |> collect
     y = 1:5 |> collect
-    rng = 555
-    Random.seed!(rng)
-    perm = Random.randperm(5)
-    @test MLJBase.shuffle_rows(x, y; rng=rng) == (x[perm], y[perm])
-    y = randn(5,5)
-    @test MLJBase.shuffle_rows(x, y; rng=rng) == (x[perm], y[perm,:])
-    @test MLJBase.shuffle_rows(z, y; rng=rng) == (z[perm,:], y[perm,:])
-    @test MLJBase.shuffle_rows(x, x; rng=rng) == (x[perm], x[perm])
+    
+    # In the following tests it is crucial to recreate a new `RNG` each time.
+    perm = Random.randperm(StableRNG(11900), 5)
+    @test MLJBase.shuffle_rows(x, y; rng=StableRNG(11900)) == (x[perm], y[perm])
+    y = randn(StableRNG(11900), 5, 5)
+    @test MLJBase.shuffle_rows(x, y; rng=StableRNG(11900)) == (x[perm], y[perm, :])
+    @test MLJBase.shuffle_rows(z, y; rng=StableRNG(11900)) == (z[perm,:], y[perm, :])
+    @test MLJBase.shuffle_rows(x, x; rng=StableRNG(11900)) == (x[perm], x[perm])
+end
+
+@testset "init_rng" begin
+    rng = 129
+    @test MLJBase.init_rng(rng) == Random.MersenneTwister(rng)
+    rng = StableRNG(129)
+    @test MLJBase.init_rng(rng) == rng
+    rng = -20
+    @test_throws ArgumentError MLJBase.init_rng(rng)
 end
 
 @testset "unwind" begin