add rand(::Tuple) #25278
add rand(::Tuple) #25278
Conversation
rfourquet
added
performance
|
Interesting collision! :) Something of an analog to the crux of #16029 (recapped in the "tour of the issues" section's first paragraph in #24595 (comment)), which is one of the primary motivations for moving towards #24595. Best! |
|
Thanks for making this connection! Which reminded me of another possibility to avoid this collision: not having a default type (currently I think generating a random value from a tuple would be a useful addition, which is not urgent in itself (could be 1.x), but working-around the collision could involve having to make a breaking change now (like e.g. having |
|
I would say this is already technically a collision: (since |
|
I think we'll have to go through a process where |
|
From triage: deprecate |
To be clear: only deprecate
deprecating |
|
Deprecating both |
|
Bump, I need a bit more information to finish this PR (cf. my last comment above). I would also be in favor to deprecate |
|
It seems to me we should only deprecate |
|
Requires the deprecation to be released first, so moving this to 1.x. |
|
funny fact: this API conflict was already spotted 3 and a half years ago! |
rfourquet
force-pushed
the
rf/rand/fromtuples
branch
from
4c9000d
to
1594a1c
Compare
|
This is now rebased and should be good for merging. But I'm not clear on how to update the NEWS file, which has only a v1.0.0 title (not a v1.1.0 one). |
|
|
||
| ### 2 | ||
|
|
||
| Sampler(RNG::Type{<:AbstractRNG}, t::Tuple{A,B}, n::Repetition) where {A,B} = |
There was a problem hiding this comment.
I think that at one point, this signature was in some cases less efficient than Sampler(::Type{RNG}, t::Tuple{A,B}, n::Repetition) where {RNG<:AbstractRNG,A,B}, can there still be occasional observable performance differences ?
rfourquet
force-pushed
the
rf/rand/fromtuples
branch
from
1594a1c
to
c15a86c
Compare
|
Should this be |
I don't believe so. Here the proposal is to have |
rfourquet
force-pushed
the
rf/rand/fromtuples
branch
from
c15a86c
to
3ba74d6
Compare
|
News added, I will end up merging this soon (was already triaged). |
|
There was an (hopefully) unrelated error on Appveyor with the Profile test, if someone is interested in investigating: https://ci.appveyor.com/project/JuliaLang/julia/builds/20680685/job/0krm1ke8dp4dwcfc |
changes between Julia 1.0 and 1.1, including: - Custom .css-style for compat admonitions. - Information about compat annotations to CONTRIBUTING.md. - NEWS.md entry for PRs #30090, #30035, #30022, #29978, #29969, #29858, #29845, #29754, #29638, #29636, #29615, #29600, #29506, #29469, #29316, #29259, #29178, #29153, #29033, #28902, #28761, #28745, #28708, #28696, #29997, #28790, #29092, #29108, #29782 - Compat annotation for PRs #30090, #30013, #29978, #29890, #29858, #29827, #29754, #29679, #29636, #29623, #29600, #29440, #29316, #29259, #29178, #29157, #29153, #29033, #28902, #28878, #28761, #28708, #28156, #29733, #29670, #29997, #28790, #29092, #29108, #29782, #25278 - Documentation for broadcasting CartesianIndices (#30230). - Documentation for Base.julia_cmd(). - Documentation for colon constructor of CartesianIndices (#29440). - Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix). - Run NEWS-update.jl. Co-authored-by: Morten Piibeleht <[email protected]> Co-authored-by: Fredrik Ekre <[email protected]>
changes between Julia 1.0 and 1.1, including: - Custom .css-style for compat admonitions. - Information about compat annotations to CONTRIBUTING.md. - NEWS.md entry for PRs #30090, #30035, #30022, #29978, #29969, #29858, #29845, #29754, #29638, #29636, #29615, #29600, #29506, #29469, #29316, #29259, #29178, #29153, #29033, #28902, #28761, #28745, #28708, #28696, #29997, #28790, #29092, #29108, #29782 - Compat annotation for PRs #30090, #30013, #29978, #29890, #29858, #29827, #29754, #29679, #29636, #29623, #29600, #29440, #29316, #29259, #29178, #29157, #29153, #29033, #28902, #28878, #28761, #28708, #28156, #29733, #29670, #29997, #28790, #29092, #29108, #29782, #25278 - Documentation for broadcasting CartesianIndices (#30230). - Documentation for Base.julia_cmd(). - Documentation for colon constructor of CartesianIndices (#29440). - Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix). - Run NEWS-update.jl. Co-authored-by: Morten Piibeleht <[email protected]> Co-authored-by: Fredrik Ekre <[email protected]>
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including: - Custom .css-style for compat admonitions. - Information about compat annotations to CONTRIBUTING.md. - NEWS.md entry for PRs #30090, #30035, #30022, #29978, #29969, #29858, #29845, #29754, #29638, #29636, #29615, #29600, #29506, #29469, #29316, #29259, #29178, #29153, #29033, #28902, #28761, #28745, #28708, #28696, #29997, #28790, #29092, #29108, #29782 - Compat annotation for PRs #30090, #30013, #29978, #29890, #29858, #29827, #29754, #29679, #29636, #29623, #29600, #29440, #29316, #29259, #29178, #29157, #29153, #29033, #28902, #28878, #28761, #28708, #28156, #29733, #29670, #29997, #28790, #29092, #29108, #29782, #25278 - Documentation for broadcasting CartesianIndices (#30230). - Documentation for Base.julia_cmd(). - Documentation for colon constructor of CartesianIndices (#29440). - Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix). - Run NEWS-update.jl. Co-authored-by: Morten Piibeleht <[email protected]> Co-authored-by: Fredrik Ekre <[email protected]>
PR based on #24874, only 2 new commits here.
Knowing the length a compile time gives a big advantage to something like
rand(('a', 'b'))overrand(['a', 'b'])(many times faster). This PR implements optimizations for tuples up to length 4, but it could easily be extended. The second commit here shows a use case for this, and I also need that occasionally in my programs.But this raise one question: what to do for
rand((1, 2, 3)). Currently, this calls the array contructor. So it would be a bit inconsistent to haverand(::Tuple)give a scalar except when the tuple is of typeDims. I see few possiblities about this:Dimsa dedicated types, instead of an alias for tuples ofInt. This is a big change, unlikely to happen for 1.0. But I guess most of the time you get aDimsobject by callingsizeon an array, so it doesn't seem to be a no-go in usability.rand(Array, (1, 2, 3))orrand(1, 2, 3)for the construction of an array, i.e. if the size is specified as a tuple, you get to giveArrayas a parameter.randare disabled, in favor of aVectorconstructor (with aRanditerator).randto require specifying the type of generated value (currentlyFloat64is the default) (EDITED)Thoughts?