Allow noise on sparse simulator

[daxfohl]

Fixes #2432

Adds optional noise parameter to Simulator.__init__. If this is deemed a valid approach, we can do this with the other simulators too.

Note, that's a big if. I don't have enough background to understand whether this approach makes sense.

[95-martin-orion]

Commented on #2432: certain types of noise must be rejected by the sparse simulator. This will need tests for each type of noise described in that comment (current test covers "noise gates" only).

[95-martin-orion]

@95-martin-orion Can I get a sanity check whether these decompositions look correct?

Based on the test, it looks like a global phase difference is introduced in decomposition. This isn't inherently bad behavior, but that test in particular requires global phase to be preserved.

I think the correct way to handle this is to pass a custom keep value to the simulator's decompose call, such that it only decomposes non-GateOperation objects. (Default decompose behavior tries to reduce gates to a device-specific set of gates, but that isn't necessary in simulation.) An example:

# Prevents GateOperations from being decomposed further.
cirq.decompose(moment, keep= lambda x: isinstance(x, cirq.GateOperation))

[daxfohl]

Got it, thanks!

[daxfohl]

@95-martin-orion so I replaced decompose with flatten_op_tree and now everything seems to work exactly as I'd expect. There's no need to explicitly handle global phase or anything (and we don't want to get into keep here because the whole _act_on_ initiative was introduced partially to get rid of keep and on_stuck). So it makes sense to me that this would be the "right" way to do it, and is consistent with how we can add _act_on_ to density matrix simulator in #3841.

I'm actually ready to call this PR "done" from my perspective. I know there's discussion around doing some pre-analysis of the noise, but a few things there. First, I think it's going to be cumbersome, as it would require a whole design, implementation, and integration of a can_act_on protocol. Second, it requires iterating through the whole circuit twice, once to do can_act_on and then to do act_on, which would slow things down in the nominal case, and that logic would have to be duplicated in every noise-accepting simulator. Third, we don't currently do a pre-check that circuits are simulateable, so why would we make it a requirement to do so for noise?

Either way, do you have an example of what would be channel noise and mixture noise, so I can add test cases for them? IDK really what those terms mean.

[95-martin-orion]

[...] we don't currently do a pre-check that circuits are simulateable, so why would we make it a requirement to do so for noise?

This is a fair point. I'd say the primary difference is that noise type is easier to identify than general simulability, as each gate can be checked for noise type independently of the rest of the circuit.

The motive to do this check for noise is fail-fast behavior: since large simulations can take a long time (on the order of minutes), we try to pre-check wherever possible. I don't think a noise check would represent a significant fraction of the simulation time, but if that's what you're seeing then we can leave it up to the simulator to identify the error.

Either way, do you have an example of what would be channel noise and mixture noise, so I can add test cases for them?

Sure. As a preface: all gates can be represented as mixtures and all mixtures can be represented channels, but these relations don't work in reverse. As a result, Cirq will accept e.g. cirq.channel(some_mixture), but cirq.mixture(some_channel) gives an error.

From cirq/ops/common_channels.py (an unfortunate name, since it contains both channels and mixtures):

• DepolarizingChannel is a mixture (and by extension, also a channel) since it implements _mixture_.
• AmplitudeDampingChannel is a channel (but not a mixture) since it only implements _channel_, not _mixture_.

It should be possible to use these directly as the noise argument to cirq.Simulator. (QM note: these channels represent qubit depolarization - essentially random, unexpected gates - and decay from |1> to |0>, respectively)

[daxfohl]

Sounds good. One other thing I wanted to check: in the run function, the first thing it does is split into unitary and non-unitary, so that it only runs the unitary section once if repetitions > 1. But it does this on the input circuit without taking noise into account. Will the presence of gate or mixture noise invalidate this logic, or can those be considered unitary?

[95-martin-orion]

[...] Will the presence of gate or mixture noise invalidate this logic, or can those be considered unitary?

Gate noise is (generally) unitary; mixture and channel noise is not.

In general, anything for which cirq.has_unitary returns True will produce unitary noise. Mixtures and channels return False because they're nondeterministic - there's no unitary representation for their behavior.

[daxfohl]

@95-martin-orion I think this is ready for re-review.

[95-martin-orion]

_has_(unitary|mixture)_ on the noise model fits just right. LGTM!

[daxfohl]

@smitsanghavi @tonybruguier if you want to add noise support to clifford or mps simulators, this PR can be a template to do so. It ended up being only a couple lines of code.

[tonybruguier]

@daxfohl Sent #3857 for review.

[smitsanghavi]

My understanding is that the Clifford simulator should not support any noise. Unless there is a class of noise models that have a stabilizer effect that I'm not aware of. @Strilanc to confirm.

[mpharrigan]

will this work with GateSubstitutionNoiseModel https://github.com/quantumlib/Cirq/blob/master/cirq/devices/noise_model.py#L247 for simulating coherent error (like an overrotation or something)

[daxfohl]

The noise model in sparse simulator has to have a unitary or mixture representation. Passing in a noise model without has_unitary or has_mixture will trigger a ValueError at construction time, by design.

GateSubstitutionNoiseModel uses an arbitrary Callable mapping function, and I don't think any information about unitary or mixture can be provided, guaranteed, or calculated on a regular Callable. So this can't work by default.

However one easy workaround would be to create a child class for your particular gate substitution noise model, and hard-code that to provide the has_unitary and/or has_mixture values required.

[daxfohl]

or provided

Actually, you could add an optional constructor arguments is_unitary / is_mixture to GateSubstitutionNoiseModel and allow the user to specify whether their Callable should be considered unitary / mixture. This may be the better user experience, though perhaps it's too much of a foot-gun?

[mpharrigan]

So it only needs has_unitary, not unitary? In that case it should be easy to make work in one of the ways you suggested