PauliMeasurementGate ignores the sign of the observable given to it

[Strilanc]

The following code should success but it fails:

import cirq
obs = cirq.DensePauliString("Z", coefficient=-1)
c = cirq.Circuit(
    cirq.PauliMeasurementGate(obs, key='out').on(cirq.LineQubit(0)),
)
assert cirq.Simulator().sample(c)['out'][0] == 1

The issue is that the line self._observable = tuple(observable) in the init method is ignoring the coefficient. The coefficient has to be +1 or -1 to be valid.

This is a very bad bug if you are e.g. multiplying overlapping observables together to get composite measurements, as the result of doing so can be negative. For example XX * ZZ = -YY.

[tanujkhattar]

Similar bugs at other places due to constructor accepting Iterable[Pauli] and ignoring the sign.

Another bug due to a similar issue, where the constructor expects iterable of a pauli and hence the sign gets ignored.

>>> cirq.DensePauliString(-1j *  cirq.Z) == cirq.DensePauliString('Z', coefficient=-1j)
False

Similarly, passing a dense pauli string to itself also resets the coefficient. i.e.

>>> obs = cirq.DensePauliString('Z', coefficient = -1j)
>>> cirq.DensePauliString(obs) == obs
False

Solution for this bug:

My proposal to fix this bug would be:

• Expect a Union['cirq.BaseDensePauliString', Iterable['cirq.Pauli']] instead of Iterable['cirq.Pauli'] as the observable and make self_observable by default a dense pauli string so we retain the coefficient information (also assert that coefficient is +1/-1).
• Update _decompose_ to include a invert_mask=True in the decomposed measurement gate if coefficient is -1.

Note that this would be a breaking change because

• Json serialization would be updated (serialized observable would now include the sign).
• dense pauli strings with coefficient != +1/-1 are currently accepted (coefficient is ignored) but would raise an error after the change.

If we wish to avoid a breaking change, then we'd have to include an additional coefficient parameter to the constructor, but then it allows us to do ugly things like cirq.PauliMeasurementGate(cirq.DensePauliString('Z', coefficient=-1), coefficient = -1) -- should this measure Z or -Z ?

LMK your thoughts. I'll also bring this up in the upcoming cirq sync and will send a PR for the agreed upon version.