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

Replace the use of randbool() #9569

Merged
merged 1 commit into from Jan 3, 2015

Conversation

5 participants
@ViralBShah
Copy link
Member

commented Jan 3, 2015

Deprecate randbool (#9105).
For a random Bool, instead of randbool(), use rand(Bool)
Instead of randbool(dims), use bitrand(dims)

@ViralBShah ViralBShah force-pushed the vs/randbool branch from 0796c87 to b52f58e Jan 3, 2015

@ViralBShah

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2015

Cc: @carlobaldassi

As discussed in #9105, I have only addressed randbool here. I will do bittrues and bitfalses in a different PR.

@@ -259,3 +259,5 @@ const base64 = base64encode

@deprecate sizehint(A, n) sizehint!(A, n)

@deprecate randbool() rand(Bool)
@deprecate randbool(x...) bitrand(x)

This comment has been minimized.

Copy link
@tkelman

tkelman Jan 3, 2015

Contributor

why are these different?

This comment has been minimized.

Copy link
@rfourquet

rfourquet Jan 3, 2015

Contributor

Because randbool() produces a Bool, whereas bitrand() produces a BitArray (as it should).

This comment has been minimized.

Copy link
@rfourquet

rfourquet Jan 3, 2015

Contributor

the forwarding of randbool taking an AbstractRNG won't work as is, nor randbool((2,3)).

This comment has been minimized.

Copy link
@tkelman

tkelman Jan 3, 2015

Contributor

Oh, you're right, did not realize randbool() with no inputs gave a Bool, nevermind. Glad we're deprecating that.

This comment has been minimized.

Copy link
@ViralBShah

ViralBShah Jan 3, 2015

Author Member

@rfourquet Thanks. I have added all the deprecation cases, and they work correctly now.

@ViralBShah ViralBShah force-pushed the vs/randbool branch from b52f58e to 66ec067 Jan 3, 2015

@deprecate randbool(dims) bitrand(dims)
@deprecate randbool(dims...) bitrand(dims)
@deprecate randbool(r, dims) bitrand(r, dims)
@deprecate randbool(r, dims...) bitrand(r, dims)

This comment has been minimized.

Copy link
@rfourquet

rfourquet Jan 3, 2015

Contributor

randbool(1, 2, 3) won't work, and randbool(MersenneTwister()) won't work as expected, I think you can't avoid using type annotations here!

This comment has been minimized.

Copy link
@ViralBShah

ViralBShah Jan 3, 2015

Author Member

I keep losing track of all the different method signatures!

This comment has been minimized.

Copy link
@ViralBShah

ViralBShah Jan 3, 2015

Author Member

Do we not have rand(r::AbstractRNG, t::Type) so that different RNGs can be used to generate random values of various integer/boolean types? It appears not.

This comment has been minimized.

Copy link
@ViralBShah

ViralBShah Jan 3, 2015

Author Member

Never mind. We do have it - but I was trying in 0.3.4, which was open in another window.

@ViralBShah

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2015

The linux failure is a segfault, and appears unrelated (but worrisome).

@tkelman

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2015

The linux failure is a segfault, and appears unrelated (but worrisome).

I used almost the exact same wording in another PR, though when it happened on OSX. Probably related to #9544 #9570, the ccall(:raise) might be getting caught by some signal handler inside the LLVM JIT and breaking things maybe?

@ViralBShah ViralBShah force-pushed the vs/randbool branch from 66ec067 to e8eb7e1 Jan 3, 2015

@ViralBShah

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2015

Now it happened in OS X here...

Deprecate randbool (#9105)
For a random Bool, instead of randbool(), use rand(Bool)
Instead of randbool([rng], dims), use bitrand([rng], dims)
Add randbool deprecation to NEWS

@ViralBShah ViralBShah force-pushed the vs/randbool branch from e8eb7e1 to ff69e6f Jan 3, 2015

ViralBShah added a commit that referenced this pull request Jan 3, 2015

Merge pull request #9569 from JuliaLang/vs/randbool
Replace the use of randbool()

@ViralBShah ViralBShah merged commit b365c36 into master Jan 3, 2015

0 of 2 checks passed

continuous-integration/appveyor Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci The Travis CI build is in progress
Details

@ViralBShah ViralBShah deleted the vs/randbool branch Jan 3, 2015

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented on ff69e6f Mar 15, 2015

I'm skeptical of whether we need bitrand. You can use rand! on a BitArray, or maybe we could use rand(Bool, ...) for this.

This comment has been minimized.

Copy link
Member

replied Mar 16, 2015

The trouble with having rand(Bool, ...) is that for every other T we have rand(T, ...) returning an Array{T}. There was a pretty long discussion about this.

This comment has been minimized.

Copy link
Member Author

replied Mar 16, 2015

Ok to just have rand!(BitArray), but the other proposal is not a good idea as @StefanKarpinski already mentioned.

jkbest2 added a commit to jkbest2/Distributions.jl that referenced this pull request May 2, 2016

Change randbool to rand(Bool) in vonmises sampler
As per JuliaLang/julia#9569, `randbool()` has been deprecated in favor of `rand(Bool)`.

ararslan added a commit to JuliaStats/Distributions.jl that referenced this pull request Jul 19, 2016

Change randbool to rand(Bool) in vonmises sampler (#491)
As per JuliaLang/julia#9569, `randbool()` has been deprecated in favor of `rand(Bool)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.