Skip to content
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

feature request rand support Range not just Range1 #5041

Closed
ggggggggg opened this issue Dec 6, 2013 · 5 comments · Fixed by #5059
Closed

feature request rand support Range not just Range1 #5041

ggggggggg opened this issue Dec 6, 2013 · 5 comments · Fixed by #5059
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@ggggggggg
Copy link
Contributor

julia> rand(1:10)
5
julia> rand(1:2:10)
ERROR: no method rand(Range{Int64},)

This happens because rand(r::Range1{Int64}) is defined, but rand(Range{Int64},) is not. It seems fairly straightforward to extend rand to support Range. Part of the implementation is here. For example

julia> function rand_ranges(r::Ranges{Int64})
           ulen = convert(Uint64, length(r))
           convert(Int64, first(r) + Base.Random.randu(ulen)*step(r))
       end
rand_ranges (generic function with 1 method)
julia> rand_ranges(1:10)
8
julia> rand_ranges(1:2:10)
3

seems to work. It seems like the ideal way would be to define based on 'Ranges' and it should work for both 'Range1' and 'Range'.

@timholy
Copy link
Member

timholy commented Dec 6, 2013

If you could finish this up and submit as a pull request, that would be great. See https://github.com/JuliaLang/julia/blob/master/CONTRIBUTING.md.

@JeffBezanson
Copy link
Member

In #1135 there was brief discussion of something like

rand(v::AbstractVector) = v[rand(1:length(v))]

but I don't know how everybody feels about that.

@ggggggggg
Copy link
Contributor Author

I'll try to work on this after I get installing from source to work.
Does AbstractArray necessarily support linear indexing? Would rand(v::AbstractArray) = v[rand(1:length(v))] work?
What about adding rand(Bool)=randbool() and one with dims also to make the interface a bit more uniform?

@ggggggggg
Copy link
Contributor Author

My pull request covers just Ranges, since I'm new here and I didn't want to do any more than I was specifically asked to do. But If there is agreement, I'd like to work on rand a bit more. Specifically I'm thinking the interface is mostly like

rand(::T) = random of type T
rand(::T, dims) = random array of type T
rand!(Array{T}) = fill array with random elements of type T
rand(LinearlyIndexableCollection) = random element from collection

So probably it would be mostly Jeff's function and rand(Bool) and some benchmarks to see if the more specific code is any faster than more general code. Open to guidance.

@StefanKarpinski
Copy link
Member

Yes, an AbstractArray, a supports Int indices from 1:size(a,k) in dimension k and a[i] means a[mod(i-1,size(a,1))+1,...], so linear indexing is always meaningful. Whether it's efficient is a different story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants