golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.83k stars 17.51k forks source link

math/rand/v2: revised API for math/rand #61716

Closed rsc closed 9 months ago

rsc commented 1 year ago

Based on earlier discussions in #60751, #26263, and #21835, as well as discussions with @robpike, I propose adding a new version of math/rand, imported as math/rand/v2, to the standard library. The full text is below but is nearly identical to what we discussed in #60751.

The most direct motivation for this proposal is to clean up math/rand and fix these many lingering problems, especially the use of an outdated generator, slow algorithms, and the unfortunate collision with crypto/rand.Read.

A more indirect but equally important motivation is to set an example for other v2 packages in the standard library. Creating math/rand/v2 will let us work out tooling issues (support for v2 packages in gopls, goimports, and so on) in a relatively rarely used package with fairly low stakes attached before moving on to more commonly used, higher-stakes packages like sync/v2 or encoding/json/v2.

This proposal establishes the pattern for v2 packages in the standard library, namely that they are subdirectories of the original package and that the starting point for the API is the original package, with each deviation clearly justified. In particular, v2 packages are not an opportunity to redesign something from scratch "just because". The existing APIs work well, and deviations should be carefully considered. A from-scratch new API is just as likely (probably more likely) to introduce new problems as it is to fix existing ones.

Once math/rand/v2 has shipped, we would tag and delete x/exp/rand. This would keep programs that already use x/exp/rand working (by reference to the tagged version or older versions of x/exp) but allow us to delete the code from the main branch of x/exp, making clear that development on it has ended.

A prototype of these math/rand/v2 changes can be found at https://go.dev/cl/502506 and the CLs below it in the stack.

Proposal

The math/rand/v2 API would use math/rand as a starting point and then make the following backwards incompatible changes:

  1. Remove Rand.Read and top-level Read. It is almost always a mistake to pretend that a pseudo-random generator is a good source of arbitrarily long byte sequences. The kinds of simulations and non-deterministic algorithms for which math/rand is a good choice almost never need byte sequences. Read is the only common piece of API between math/rand and crypto/rand, and code should essentially always use crypto/rand.Read instead. (math/rand.Read and crypto/rand.Read are problematic because they have the the same signature; math/rand.Int and crypto/rand.Int also both exist, but with different signatures, meaning code never accidentally mistakes one for the other.)

  2. Remove Source.Seed, Rand.Seed, and top-level Seed. Top-level Seed is deprecated as of Go 1.20. Source.Seed and Rand.Seed assume that the underlying source can be seeded by a single int64, which is only true of a limited number of sources. Specific source implementations can provide Seed methods with appropriate signatures, or none at all for generators that cannot be reseeded; the details of seeding do not belong in the general interface.

    Note that removing top-level Seed means that the top-level functions like Int will always be randomly seeded rather than deterministically seeded. math/rand/v2 will not pay attention to the randautoseed GODEBUG setting that math/rand does; auto-seeding for top-level functions is the only mode. This means in turn that the specific PRNG algorithm used by the top-level functions is unspecified and can change from release to release without breaking any existing code.

  3. Change the Source interface to have a single Uint64() uint64 method, replacing Int63() int64. The latter was overfitting to the original Mitchell & Reeds LFSR generator. Modern generators can provide uint64s, and uint64s have fewer special cases at call sites.

  4. Remove Source64, which is unnecessary now that Source provides the Uint64 method.

  5. Use a more straightforward implementation in Float32 and Float64. Taking Float64 as an example, it originally used float64(r.Int63()) / (1<<63), but this has the problem of occasionally rounding up to 1.0, which Float64 must not. We tried changing it to float64(r.Int63n(1<<53) / (1<<53), which avoids the rounding problem, but we decided it was a backwards compatibile change to break the value streams that rand.Rand generators, and instead added a retry loop for the rare 1.0 case. Now we can make the breaking change, which is simpler and faster.

    Note that some people have observed that the simple division does not make use of all the possible float64 values in the range [0, 1). For example values like 1/(1<<54), 1/(1<<55), 3/(1<<55), and so on are not generated at all, both in today's math/rand and in this simpler algorithm. Only the values 0 and 1/(1<<53) are, not the ones in between. It is possible to introduce even more complex algorithms to spread out the low values more while preserving something that can be deemed a uniform distribution, but these algorithms seem like overkill. The simple division should continue to suffice.

  6. (New since #60751.) Correct bias problems in ExpFloat64 and NormFloat64. @flyingmutant reports that these have detectable bias, perhaps due to a mistake in porting the Ziggurat algorithms from float32 to float64. We can take this opportunity to correct those.

  7. Implement Rand.Perm in terms of Rand.Shuffle. Shuffle is a bit more efficient, and then we have only one implementation. It has been suggested elsewhere to remove Rand.Perm instead, but keeping it costs little (especially if implemented in terms of Shuffle) and avoids unnecessary churn for users.

  8. Rename Intn, Int31, Int31n, Int63, Int64n to IntN, Int32, Int32N, Int64, Int64N. The 31 and 63 in the names are unnecessarily pedantic and confusing, and the capitalized N is more idiomatic for a second "word" in the name in Go. Note: The capital N's in the names are a change from #60751.

  9. Add Uint32, Uint32N, Uint64, Uint64N, Uint, UintN, both as top-level functions and methods on Rand. At the least, we need Uint64 to provide access to the widest values that the Source can provide. But we may as well complete the set while we are here. Note: The capital N's in the names are a change from #60751.

  10. (New since #60751) Add a generic top-level function N that is like Int64N or Uint64N but works for any integer type. In particular this allows using rand.N(1*time.Minute) to get a random duration in the range [0, 1*time.Minute).

  11. Use Lemire's algorithm in N, IntN, UintN, and so on. Preliminary benchmarks show a 40% savings compared to v1 Int31n and a 75% savings compared to v1 Int63n. (Like with Float64, since this changes the values returned, it is a breaking change that can only be applied in math/rand/v2.)

  12. Add a new Source implementation, PCG-DXSM, with this API:

     func NewPCG(seed1, seed2 uint64) *PCG
     type PCG struct { ... }
     func (p *PCG) Uint64() uint64
     func (p *PCG) Seed(seed1, seed2 uint64)
     func (p *PCG) MarshalBinary() ([]byte, error)
     func (p *PCG) UnmarshalBinary(data []byte) error

    PCG is a simple, efficient algorithm with good statistical randomness properties. The DXSM variant was introduced by the author specifically to correct a rare, obscure shortcoming in the original (PCG-XSLRR) and is now the default generator in Numpy.

    PCG is provided for people who are writing simulations and need a seeded, deterministic source of randomness suitable for most purposes. Note that PCG is not assumed in any code. In particular the top-level functions need not use PCG (and in the current prototype do not). If in the future we add another algorithm, it will sit alongside PCG as a true peer.

    Note: MarshalBinary and UnmarshalBinary are new since #60751.

  13. Remove the Mitchell & Reeds LFSR generator and NewSource. An alternative to removing it would be to give it a name like NewLFSR or NewMitchellReeds, but that would serve no purpose. The obvious purpose would be to provide the original math/rand source so that code that needs to reproduce the exact math/rand outputs can do so. But the other changes to Rand, in routines like Intn and Float64, change the derived values from Rand's methods for a given Source input stream. Since preserving the math/rand Source stream would not suffice to preserve the math/rand Rand stream, preserving the math/rand Source is not worthwhile. Programs that need exactly the math/rand value streams can continue to use that package; it will not be removed.

  14. (New since #60751 and the original proposal in this issue) Add a new Source implementation, ChaCha8, with this API:

     func NewChaCha8(seed [32]byte) *ChaCha8
     type ChaCha8 struct { ... }
     func (c *ChaCha8) Uint64() uint64
     func (c *ChaCha8) Seed(seed [32]byte)
     func (c *ChaCha8) MarshalBinary() ([]byte, error)
     func (c *ChaCha8) UnmarshalBinary(data []byte) error

    ChaCha8 is a cryptographically-strong random number generator derived from the ChaCha8 stream cipher. It provides security equivalent to ChaCha8 encryption.

  15. (New since #60751 and the original proposal in this issue) Use a per-OS-thread ChaCha8 for the global random generator, both in math/rand/v2 and in math/rand (when unseeded). The current implementation uses a per-OS-thread wyrand, which was fine compared to the Mitchell/Reeds/Thompson generator but is not really great compared to PCG or ChaCha8. In particular, it is strange for the top-level randomness to be less studied and less robust than the two concrete implementations provided. Using ChaCha8 also gives the default top-level randomness real cryptographic strength, turning accidental use of math/rand where crypto/rand would be more appropriate into less of a security problem. Using ChaCha8 runs slower than using wyrand, but it is about as fast as the current sync.Mutex-locked generator in math/rand (~6ns per value on my 2019 Intel MacBook Pro).

Differences since previous discussion

The differences between this proposal and what we discussed in #60751 are:

These differences are also marked above.

Discussion Summary

Summarizing the discussion on #60751:

jdemeyer commented 11 months ago

It would be good to explicitly document which guarantees there are (or aren't) that output won't change between Go versions. The old math/rand seemed to have an implicit promise that random values wouldn't change (even preserving the inefficient Float64 algorithm because of this), until Go 1.20 came along and decided that the global functions would be seeded randomly.

Merovius commented 11 months ago

@jdemeyer I think the proposal is pretty clear, that there is no guarantee for the per-OS-thread source (given that goroutines are scheduled freely to OS-threads, that doesn't even seem possible). I also think it's pretty clear that the individual Source implementations will produce stable streams for the same seed.

I think the one thing that requires a clear statement is if a *rand.Rand produces the same values from all its methods, when used with the same Source. I would assume it is guaranteed, as that's the recommended way to get reproducible simulations. If so, the algorithm used for the Intn method (for example) can not change in future Go versions, though the algorithm used for the Intn function can (as the default source is not guaranteed to produce a stable stream anyways).

rsc commented 11 months ago

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

mateusz834 commented 11 months ago

I also wonder what will happen when the getrandom syscall blocks (because there is no entropy), crypto/rand prints an error to stderr in this case:

https://github.com/golang/go/blob/638e0d36d2a6b2efce0550978bf48e7df1166a0b/src/crypto/rand/rand_unix.go#L44-L46C2

gopherbot commented 11 months ago

Change https://go.dev/cl/532756 mentions this issue: math/rand/v2: add ChaCha8 doc comments and binary methods

seebs commented 9 months ago

Was recently running some tests that used some random data. They genuinely didn't care what the data was, just wanted it to be anything at all not just zeros. The switch from math/rand.Read to crypto/rand.Read made that go from <1% of CPU time to, I think, around 10% of CPU time used by the entire test run.

I think my intuition here is that the arguments against using a PRNG as a random data source are much weaker when talking about test cases than they are when talking about production usage. But also, I think it's important to note that at least on some systems (I was hitting this on a Mac), the performance penalty for using the crypto reader is pretty devastating.

It really feels like "I genuinely don't care if the data's any good, I just want a few KB of bits that aren't completely obviously patterned" is a real use case. On the other hand, it's probably easy enough to synthesize it from Uint64(), and that's probably equivalent to what's happening internally anyway.

I just want to stress, though, that the performance penalty is not uniformly trivial on at least some current platforms.

rsc commented 9 months ago

These changes are now all submitted for Go 1.22.