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

math/rand: rand.Float{32,64}() can erroneously return 1 #6721

Closed
gopherbot opened this issue Nov 6, 2013 · 12 comments
Closed

math/rand: rand.Float{32,64}() can erroneously return 1 #6721

gopherbot opened this issue Nov 6, 2013 · 12 comments
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 6, 2013

by akalin:

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. call rand.Float32() a lot of times

What is the expected output?

It should always return a number < 1.

What do you see instead?

Sometimes, it returns 1. See http://play.golang.org/p/BYLctrSX4B

Which compiler are you using (5g, 6g, 8g, gccgo)?

not sure, but it repros on play.golang.org

Which operating system are you using?

OS X

Which version are you using?  (run 'go version')

go version go1.1.2 darwin/amd64

Looking at the source of rand.Float32() there are a few problems:

- Float64() is basically float32(rand.Float64()), so it's possible that if the 64-bit
float is really close to 1, it might get rounded to 1 when cast (I think).
- Float64() does float64(r.Int63()) / (1 << 63), but float64s only have 52+1
fractional bits, so if the returned int63() is close to 2**63, then doing the division
may round to 1.

A few solutions:
- Change the docs to say that the returned float is in [0, 1] instead of [0, 1), but the
code probably has to be changed anyway since there's probably a bias against getting 1.
- Loop until the generated number is < 1.
- Take the high bits of Int63() and stuff them into the fractional bits, setting the
exponent to -1.
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 6, 2013

Comment 1:

Labels changed: added priority-later, go1.3, removed priority-triage.

Status changed to Accepted.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 6, 2013

Comment 2 by jeff.allen:

Float32() is just float32(r.Float64())
This happens on play.golang.org on call number 7533753 to Float64, which returns
0.9999999777168924 (bits: 11111111101111111111111111111111110100000010010110111011011010)
http://play.golang.org/p/tGq_lqQIeL
Does seem like a better solution is to use Int32 to get mantissa bits and set the
exponent, then use http://golang.org/pkg/math/#Float32frombits on the bits.
This technique would also allow us to skip the floating point divide in Float64().
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 6, 2013

Comment 3 by arnehormann:

An alternative solution from my response in
https://groups.google.com/forum/#!topic/golang-nuts/94bTFkFUteE
(math.Float32frombits(0x3f800000 | (0x007fffff & uint32(VALUE))) - 1.0)
use IEEE 754 exponent for range [1,2)
combine with mantissa bits of VALUE
make float32
subtract 1
I initially proposed this for float64 in
https://golang.org/issue/4965
but Remy convinced me that the current approach for float64 is superior.
Maybe it's usefull for float32.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 6, 2013

Comment 4:

I think the implementations should change to
float64(r.Int63n(1<<53)) / (1 << 53)
and
float32(r.Int31n(1<<24)) / (1 << 24)
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 6, 2013

Comment 5 by akalin:

+rsc, yeah that's roughly the solution I converged on.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Nov 7, 2013

Comment 6 by jeff.allen:

https://golang.org/cl/22730043
@davecheney
Copy link
Contributor

@davecheney davecheney commented Nov 7, 2013

Comment 7:

Status changed to Started.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 8:

Labels changed: added release-go1.3, removed go1.3.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 9:

Labels changed: added repo-main.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 18, 2013

Comment 10:

This issue was closed by revision 17dc712.

Status changed to Fixed.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented May 19, 2014

Comment 11:

CL https://golang.org/cl/95460049 mentions this issue.
@rsc
Copy link
Contributor

@rsc rsc commented May 19, 2014

Comment 12:

This issue was closed by revision 5aca051.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.