Passing a random number generator to simulator/sampler methods should VS maintaing an internal random state

[NoureldinYosri]

All Cirq simulators maintain an internal np.random.RandomState

Cirq/cirq-core/cirq/sim/sparse_simulator.py

Line 130 in e1b03ef

seed: 'cirq.RANDOM_STATE_OR_SEED_LIKE' = None,

This is fine when running in a single thread, however we are starting to have more places where use multiprocessing/multithreads (e.g. using multiprocessing.Pool, concurrent.futures.ThreadPoolExecutor, or other multiprocessing/multithreading libraries) and in these cases this internal random state negatively affects the simulations in two ways

• the internal random state becomes a shared state that causes the different threads/processes blocked on each other.
• the results of simulations become correlated.

---

Suggested solution: Start to prefer passing prngs to methods/functions over maitaining an internal state. this prng should be an np.random.Generator instead of an np.random.RandomState so that we get a spawn method to use when starting threads/processes.

related: #6531

[verult]

From Cirq Cynque: look into a way to convert between np.random.RandomState and np.random.Generator as an alternative approach.

We agreed that adding an RNG argument to each method call (with a default) is useful, but to maintain backward compatibility for the initializer RNG argument, and to make it easier to share the random distribution across method calls, we should still keep an internal RNG as the default fallback if an RNG is not passed into a method call.

[justinpan0]

Hi team,

I'm trying to address this in draft PR #6944. Here's the basic approach:

    if isinstance(seed, np.random.RandomState):
        self._prng = np.random.default_rng(seed.get_state()[1][0])
    elif isinstance(seed, np.random.Generator):
        self._prng = seed
    else:
        self._prng = np.random.default_rng(seed)

Plus, run and sample method would accept an optional RNG parameter while falling back to the internal RNG when none is provided. Please let me know any feedback or concerns about the implementation or backward compat, thanks!

[NoureldinYosri]

@justinpan0 it doesn't look like the changes in #6944 are backward compatible.

---

@pavoljuhas I think supporting generators should be part of cirq 2.0, what do you think?

[justinpan0]

@NoureldinYosri @pavoljuhas

I think we are dealing with three aspects of incompatibility:

1. The changes should be still backward compatible in the sense that the simulator can still use real random numbers for simulations.

2. Incompatibility with existing unit tests. Many unit tests in the codebase rely on hardcoded expected values for specific seeds. At least 61 of them.

3. The transition from np.random.RandomState to np.random.Generator introduces a fundamental incompatibility in the sequences of random numbers generated for the same seed. In my understanding, this is because RandomState and Generator use different algorithms internally, so the same seed does not yield the same sequence of random numbers between the two.

To work around point 3, I extracted the seed from the RandomState object using its get_state() method and used this seed to initialize a new Generator.

if isinstance(random_state, np.random.RandomState):
        return np.random.default_rng(random_state.get_state()[1][0]) # type: ignore[index]

While this aligns original RandomState with the Generator using the same seed, the sequences will still differ due to reason in point 3. And if we are changing lots of unit test all at once, it might introduce regression risks for point 2. Any insights?