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

Fix #8257 #8273

Merged
merged 1 commit into from
Sep 10, 2014
Merged

Fix #8257 #8273

merged 1 commit into from
Sep 10, 2014

Conversation

ivarne
Copy link
Sponsor Member

@ivarne ivarne commented Sep 8, 2014

This new 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
Copy link
Sponsor Member Author

ivarne commented Sep 8, 2014

Is this within the scope of @JuliaBackports?

@JeffBezanson
Copy link
Sponsor Member

Yes I think we could backport this. Could you add a test as well?

@ivarne
Copy link
Sponsor Member Author

ivarne commented Sep 8, 2014

Random and floating point inaccuracies is really hard to test. I made a simple attempt. I also changed == zero(step(r)) to == 0, because all numbers can be compared to 0.

@ivarne
Copy link
Sponsor Member Author

ivarne commented Sep 8, 2014

Sorry, the first commit was without the test.

@@ -219,17 +219,9 @@ end
rand!{T<:Union(Signed,Unsigned,Bool,Char)}(r::UnitRange{T}, A::AbstractArray) = rand!(RandIntGen(r), A)

function rand!{T}(r::Range{T}, A::AbstractArray)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change you can get rid of parametrisation by T, no?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

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 that referenced this pull request Sep 10, 2014
@ivarne ivarne merged commit 20cb87a into JuliaLang:master Sep 10, 2014
@ivarne
Copy link
Sponsor Member Author

ivarne commented Sep 10, 2014

I decided to merge this so that @rfourquet can easier base his work on this change. If nobody has objections in a few days, I will also backport to release-0.3.

@ViralBShah
Copy link
Member

+1 for backporting

ivarne added a commit that referenced this pull request Sep 15, 2014
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
@ivarne
Copy link
Sponsor Member Author

ivarne commented Sep 15, 2014

Backported in 7edddb0

@ViralBShah ViralBShah added the 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
Labels
randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants