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.

Sign up for GitHub

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

Jump to bottom

rand(::Range) computes elements incorrectly for FloatRange #8257

Closed
simonster opened this issue Sep 6, 2014 · 1 comment
Closed

rand(::Range) computes elements incorrectly for FloatRange #8257

simonster opened this issue Sep 6, 2014 · 1 comment
Labels
domain:randomness Random number generation and the Random stdlib

Comments

@simonster
Copy link
Member

rand(r::Range) just multiplies by step(r) instead of multiplying by r.step and dividing by r.divisor as in FloatRange indexing. This leads to cases where the return value is not actually in the range, e.g. rand(1:1/3:2) in 1:1/3:2 is sometimes false because rand returns 1.6666666666666665.
@ivarne
Copy link
Sponsor Member

ivarne commented Sep 8, 2014

Why do we even have a rand(r::Range) method?

rand(a::AbstractArray) seems just as appropriate, and will use getindex to avoid depending any range specific behaviour. Introducing that, seems to be the topic for #6003 and the range vs sample discussion. I'll try a quick fix on this issue instead.

ivarne added a commit to ivarne/julia that referenced this issue Sep 8, 2014
@ivarne
Fix JuliaLang#8257
128d10f 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`
ivarne added a commit to ivarne/julia that referenced this issue Sep 8, 2014
@ivarne
Fix JuliaLang#8257
c7d2782 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`

Also added tests, and fixed a small issue in `in(v, r::Range)` where two
calls to step() is not needed
ivarne added a commit to ivarne/julia that referenced this issue Sep 8, 2014
@ivarne
Fix JuliaLang#8257
b69c468 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`

Also added tests, and fixed a small issue in `in(v, r::Range)` where two
calls to step() is not needed
ivarne added a commit to ivarne/julia that referenced this issue Sep 8, 2014
@ivarne
Fix JuliaLang#8257
44020db 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`

Also added tests, and fixed a small issue in `in(v, r::Range)` where two
calls to step() is not needed
@rfourquet rfourquet mentioned this issue Sep 10, 2014
Closed
ivarne added a commit to ivarne/julia that referenced this issue Sep 10, 2014
@ivarne
Fix JuliaLang#8257
37a1a1e 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`

Also added tests, and fixed a small issue in `in(v, r::Range)` where two
calls to step() is not needed
@ivarne ivarne closed this as completed in 48f27bc Sep 10, 2014
ivarne added a commit that referenced this issue Sep 10, 2014
@ivarne
Merge pull request #8273 from ivarne/rand_range
20cb87a 
Fix #8257
rfourquet added a commit to rfourquet/julia that referenced this issue Sep 11, 2014
@rfourquet
make rand work with AbstractArray instead of only with Range
882580f 
This implements the generalization suggested by @ivarne
(JuliaLang#8257 (comment))
or by @lindahua
(JuliaLang#6003 (comment)).
This change is very simple thanks to commit 48f27bc.
rfourquet added a commit to rfourquet/julia that referenced this issue Sep 11, 2014
@rfourquet
make rand work with AbstractArray instead of only with Range
cbbe3cc 
This implements the generalization suggested by @ivarne
(JuliaLang#8257 (comment))
or by @lindahua
(JuliaLang#6003 (comment)).
This change is very simple thanks to commit 48f27bc.
ivarne added a commit that referenced this issue Sep 15, 2014
@ivarne
Fix #8257
7edddb0 
This version of `rand(r::Range)` and `rand!(r::Range,a::AbstractArray)`
uses getindex instead of trying to calculate the exact value of the
range. This is good because we avoid duplicating the getindex logic in
`FloatRange`

Also added tests, and fixed a small issue in `in(v, r::Range)` where two
calls to step() is not needed

Backport of 48f27bc
PR: #8273
rfourquet added a commit to rfourquet/julia that referenced this issue Sep 17, 2014
@rfourquet
make rand work with AbstractArray instead of only with Range
e55a697 
This implements the generalization suggested by @ivarne
(JuliaLang#8257 (comment))
or by @lindahua
(JuliaLang#6003 (comment)).
This change is very simple thanks to commit 48f27bc.
rfourquet added a commit to rfourquet/julia that referenced this issue Sep 30, 2014
@rfourquet
make rand work with AbstractArray instead of only with Range
6eda92f 
This implements the generalization suggested by @ivarne
(JuliaLang#8257 (comment))
or by @lindahua
(JuliaLang#6003 (comment)).
This change is very simple thanks to commit 48f27bc.
rfourquet added a commit to rfourquet/julia that referenced this issue Sep 30, 2014
@rfourquet
make rand work with AbstractArray instead of only with Range
6d329ce 
This implements the generalization suggested by @ivarne
(JuliaLang#8257 (comment))
or by @lindahua
(JuliaLang#6003 (comment)).
This change is very simple thanks to commit 48f27bc.
@ViralBShah ViralBShah added the domain:randomness Random number generation and the Random stdlib label Nov 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
domain:randomness Random number generation and the Random stdlib
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ivarne @simonster @ViralBShah