-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make sprand type stable #16074
make sprand type stable #16074
Conversation
Actually, maybe the second sprand function is better as: function sprand{T}(m::Integer, n::Integer, density::AbstractFloat,
rfn::Function, t::Type{T}=eltype(rfn(1)))
return sprand(GLOBAL_RNG, m, n, density, rfn, t)
end |
Added that. Edit: Removed that. The passed random function doesn't take a RNG object for the second sprand function |
3fa04ff
to
f0080a0
Compare
Interesting, |
f0080a0
to
6434092
Compare
Maybe |
I just rebased on a newer master commit |
@@ -917,7 +917,7 @@ function sprand{T}(m::Integer, n::Integer, density::AbstractFloat, | |||
rfn::Function, ::Type{T}=eltype(rfn(1))) | |||
N = m*n | |||
N == 0 && return spzeros(T,m,n) | |||
N == 1 && return rand() <= density ? sparse(rfn(1)) : spzeros(T,1,1) | |||
N == 1 && return rand() <= density ? sparse([1], [1], rfn(GLOBAL_RNG,1)) : spzeros(T,1,1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong. Will fix in a while. rfn should not take a RNG.
6434092
to
a7f3108
Compare
Test passes, ready for review / merge. |
# 16073 | ||
@inferred sprand(1, 1, 1.0) | ||
@inferred sprand(1, 1, 1.0, rand, Float64) | ||
@inferred sprand(1, 1, 1.0, x->round(Int,rand(x)*100)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why x -> round(Int, rand(x)*100)
rather than x -> rand(0:100, x)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied this from another test that happened to fail when I did a mistake in implementing this PR:
julia/test/sparsedir/sparse.jl
Line 592 in e8601e8
let S = sprand(50, 30, 0.5, x->round(Int,rand(x)*100)), I = sprandbool(50, 30, 0.2) |
also fix use of undefined variable in sprand
a7f3108
to
8159a38
Compare
Bump |
I guess we should separately have a version of |
We already do - this was just a type inference issue in the sprand case for matrices. All good. Thanks. |
fixes #16073