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

proposal: math/rand: add function for random bool #23804

Open
kirinrastogi opened this issue Feb 13, 2018 · 10 comments

Comments

@kirinrastogi
Copy link

commented Feb 13, 2018

I want to be able to call rand.Bool() and receive a pseudo-random bool when I import math/random

@kirinrastogi kirinrastogi referenced a pull request that will close this issue Feb 13, 2018
@gopherbot

This comment has been minimized.

Copy link

commented Feb 13, 2018

Change https://golang.org/cl/93517 mentions this issue: add function for bool in math/rand

@bradfitz bradfitz changed the title Add function for random bool in math/rand proposal: Add function for random bool in math/rand Feb 13, 2018

@gopherbot gopherbot added this to the Proposal milestone Feb 13, 2018

@gopherbot gopherbot added the Proposal label Feb 13, 2018

@ianlancetaylor ianlancetaylor changed the title proposal: Add function for random bool in math/rand proposal: math/rand: add function for random bool Feb 13, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2018

How about math.IntN(2) == 0? Is it really worth adding a new function for this?

@mdcnz

This comment has been minimized.

Copy link

commented Feb 13, 2018

@kirinrastogi

This comment has been minimized.

Copy link
Author

commented Feb 13, 2018

Yes I believe it is worth it. rand.Intn(2) == 0 is not as expressive as rand.Bool(). This is more clear

@JackyChiu

This comment has been minimized.

Copy link

commented Feb 13, 2018

How about math.IntN(2) == 0? Is it really worth adding a new function for this?

I'd argue that it's worth doing. It's not solving a hard problem but it's making it easier to read. Similar to how sync.Waitgroup.Done() is an just an extra 1 line function but makes it more expressive to read for the programmer.

@ALTree

This comment has been minimized.

Copy link
Member

commented Feb 13, 2018

I don't have an opinion on this, but I have a small argument in favour.

The way the rand package uses the randomness source is quite intricate. A lot of the functions do not use the generator directly, but call other functions and then manipulate the input in some way to build a value of the requested type. For this reason, a lot of the "obvious" ways to get a random bool result in very deep call stacks.

Take for example the simple method Ian proposed above: rand.Intn(2) == 0. This is what happens:

We call r.Intn(2), which checks that the argument fits an int32, and then calls r.Int31n(2), which checks that the argument is a power of two, and then calls r.Int31(), which calls r.Int63() and then shifts the result and cast it into an int32.

This is what the call stack looks like:

  math/rand.(*Rand).Int63(0xc42007a1e0, 0xff)
	math/rand/rand.go:84 +0x22
  math/rand.(*Rand).Int31(0xc42007a1e0, 0xc42002def0)
	math/rand/rand.go:101 +0x2b
  math/rand.(*Rand).Int31n(0xc42007a1e0, 0x13b2c00000002, 0x390ffd3f)
	math/rand/rand.go:133 +0xb4
  math/rand.(*Rand).Intn(0xc42007a1e0, 0x2, 0x5a832bc1)
	math/rand/rand.go:174 +0x45
  math/rand.Intn(0x2, 0x4a91e1)
	math/rand/rand.go:331 +0x37 

An implementation that directly calls the function that is the nearest to the generator: Int63() % 2 == 0, on the other hand, has a much smaller call stack, and it about 20% faster on my machine in a quick benchmark I wrote.

So while it's true that it is easy to write an expression that correctly returns a random boolean, doing so in a way that is performant and does not result in a deep call stack is not as immediate as it looks.

@bcmills

This comment has been minimized.

Copy link
Member

commented Feb 13, 2018

We call r.Intn(2), which checks that the argument fits an int32, and then call calls r.Int31n(2), which checks that the argument is a power of two, and then calls r.Int31(), which calls r.Int63() and then shifts the result and cast it into an int32.

All of those checks and conversions should be easy to inline and constant-fold away.
If they are not inlined and constant-folded, that is arguably a compiler bug, not an API bug.

@faiface

This comment has been minimized.

Copy link

commented Feb 15, 2018

If you're used to rand.Intn(2) == 0, which you are after you've used it 2-3x, it's as expressive as rand.Bool() would be.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Feb 26, 2018

Leaving this for math/rand v2 discussions. Please just use r.Intn(2) == 0 for now.

@rsc rsc added the Proposal-Hold label Feb 26, 2018

@odeke-em

This comment has been minimized.

Copy link
Member

commented Aug 25, 2018

/cc @josharian too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants
You can’t perform that action at this time.