Implement PauliStringPhasorGate

[daxfohl]

BREAKING CHANGE: PauliStringPhasor no longer inherits from PauliStringGateOperation.

Implements PauliStringPhasor in terms of a GateOperation on a new class PauliStringPhasorGate.

Mostly involved moving existing functions from the operation to the gate, and then having the operation call those methods under the hood.

Closes #1561, xref #4683

@tanujkhattar

[tanujkhattar]

A first round of high level comments. My main question is that whether we can fix these inconsistencies at a higher level; i.e.

Q1: Can we make PauliString a GateOperation ?

Q2: Can we make PauliStringGateOperation a GateOperation or deprecate it after this change?

[daxfohl]

Q1: Can we make PauliString a GateOperation ?

I didn't notice that it wasn't (it has a gate field). It would be nice if we could. I'll look into that separately. I don't think it would affect what we do in this PR, but would be good to verify. Edit: I looked at PauliString and I think it's fine as-is. Having a gate is the important thing, whether or not it inherits GateOperation.

Q2: Can we make PauliStringGateOperation a GateOperation or deprecate it after this change?

I think we should deprecate it. It doesn't appear to be a useful abstraction, it just provides some convenience functions that should probably be module-level, if anything. That said, we may want an interface ContainsPauliString that could be used for anything that has a pauli_string property.

[daxfohl]

@tanujkhattar Here's a slightly different approach where we don't break the inheritance from PauliStringGateOperation. https://github.com/quantumlib/Cirq/compare/master...daxfohl:gates-psp2?expand=1. This would be safer, I think, and follows the precedent of PauliString, in that it doesn't inherit GateOperation, but still provides a gate. (As I mentioned above, to me having a gate is the important thing; it doesn't matter whether it inherits GateOperation or not).

It's a little more redundant, since it has to do all the protocol forwarding. But safer since it doesn't break the hierarchy. I do see a couple places in the code where isinstance(PauliStringGateOperation) is used. So it's probably better to be safe here. If we want to deprecate PauliStringGateOperation later and change the hierarchy, we can do that separately. But I don't see a particular need to rush that.

(Ideally we could make PauliStringGateOperation inherit GateOperation and have the best of both worlds. But I can't think of a way to do that since it is not guaranteed to have a gate. Unless we give it a dummy NoOpGate if nothing else is provided.)

What do you think? Should we go with this alternate approach?

[daxfohl]

Per cirq-sync notes it sounds like we'd prefer the breaking change. I'll revert to the previous implementation here, and then make sure everywhere our code has isinstance(PauliStringGateOperation) also handles GateOperation[PauliStringPhasorGate].

[daxfohl]

Reverted this to bring back the hard break from PauliStringGateOperation as we'd discussed in cirq-sync noted above. @tanujkhattar

[tanujkhattar]

Looking pretty good. Left a final round of comments.

[tanujkhattar]

Final round of comments. Thanks for the iterations!

[tanujkhattar]

LGTM