Skip to content

Commit

Permalink
math/rand: improve uniformity of rand.Float64,Float32
Browse files Browse the repository at this point in the history
Replaced code that substituted 0 for rounded-up 1 with
code to try again.  This has minimal effect on the existing
stream of random numbers, but restores uniformity.

Fixes #12290.

Change-Id: Ib68f0b0a4a173339bcd0274cc16509f7b0977de8
Reviewed-on: https://go-review.googlesource.com/17670
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
dr2chase committed Dec 11, 2015
1 parent dcc821f commit 38255cb
Showing 1 changed file with 11 additions and 14 deletions.
25 changes: 11 additions & 14 deletions src/math/rand/rand.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,18 @@ func (r *Rand) Float64() float64 {
//
// There is one bug in the value stream: r.Int63() may be so close
// to 1<<63 that the division rounds up to 1.0, and we've guaranteed
// that the result is always less than 1.0. To fix that, we treat the
// range as cyclic and map 1 back to 0. This is justified by observing
// that while some of the values rounded down to 0, nothing was
// rounding up to 0, so 0 was underrepresented in the results.
// Mapping 1 back to zero restores some balance.
// (The balance is not perfect because the implementation
// returns denormalized numbers for very small r.Int63(),
// and those steal from what would normally be 0 results.)
// The remapping only happens 1/2⁵³ of the time, so most clients
// that the result is always less than 1.0.
//
// We tried to fix this by mapping 1.0 back to 0.0, but since float64
// values near 0 are much denser than near 1, mapping 1 to 0 caused
// a theoretically significant overshoot in the probability of returning 0.
// Instead of that, if we round up to 1, just try again.
// Getting 1 only happens 1/2⁵³ of the time, so most clients
// will not observe it anyway.
again:
f := float64(r.Int63()) / (1 << 63)
if f == 1 {
f = 0
goto again // resample; this branch is taken O(never)
}
return f
}
Expand All @@ -134,13 +133,11 @@ func (r *Rand) Float64() float64 {
func (r *Rand) Float32() float32 {
// Same rationale as in Float64: we want to preserve the Go 1 value
// stream except we want to fix it not to return 1.0
// There is a double rounding going on here, but the argument for
// mapping 1 to 0 still applies: 0 was underrepresented before,
// so mapping 1 to 0 doesn't cause too many 0s.
// This only happens 1/2²⁴ of the time (plus the 1/2⁵³ of the time in Float64).
again:
f := float32(r.Float64())
if f == 1 {
f = 0
goto again // resample; this branch is taken O(very rarely)
}
return f
}
Expand Down

2 comments on commit 38255cb

@fabienlefloch
Copy link

Choose a reason for hiding this comment

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

The change does not really improve uniformity that much. Again, as there are more floating points near zero, the outcome will be skewed towards zero with probability 1/2^11.
The change suggests a confusion in the definition of uniformity. Hitting all floating point numbers with equal probability does not mean uniform, because of the over representation of the numbers close to zero. So the fix continues the same mistake, just make things on average a tiny bit slower, and will be useless for Quasi random number generators.
The positive outcome however is that now floating points number are hit with equal probability (which was not the case with f=0 code). But it's not uniform, and not good for Monte Carlo simulations

@dr2chase
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have a test that we could run that would demonstrate this non-uniformity? It ought to be pretty obvious in Float32.

Also: "Hitting all floating point numbers with equal probability"
I don't think we do that.
We hit 0 and 1 with p=1/2^63.
(please forgive off-by-ones below)
We hit 1-(1/2^53) with all the points between (1-(1/2^54))2^63 and (1-3(1/2^54))*2^63
I think that is 2^10 points from the Int63 distribution mapping to a single point near one in the Float64 distribution.

Please sign in to comment.