Add XPow/YPow/ZPow and factor out CirqGateAsBloqBase
#469
Conversation
|
This LGTM % nits. The more standard rotation bloqs are preserved, which is good! |
|
should we remove/replace the existing Rx,Ry,Rz? I'm not a huge fan of the "pow" nomenclature since I don't think this has really caught on outside of Cirq; but this matches cirq so I can see why we'd want to keep the name |
|
Why would we remove them? It's convenient having the textbook rotation gates around no? |
Yeah, I don't think we should remove them. We can probably make them derive from |
… Address other nits
|
@mpharrigan @fdmalone This is ready for a re-review |
That's right, and this approach can be used for multi qubit cirq-core gates as well (like XX, YY, ZZ, etc.); the exact same code will work. |
| @@ -53,34 +54,36 @@ def _get_cirq_quregs(signature: Signature, qm: InteropQubitManager): | |||
| return ret | |||
|
|
|||
|
|
|||
| @frozen | |||
| class CirqGateAsBloq(GateWithRegisters): | |||
| class CirqGateAsBloqBase(GateWithRegisters): | |||
There was a problem hiding this comment.
don't you need to mark this as abstract? even if the metaclass is inherited, it would probably help for documenting
| gate: cirq.Gate | ||
| @property | ||
| @abc.abstractmethod | ||
| def cirq_gate(self) -> cirq.Gate: |
There was a problem hiding this comment.
update class docstring .. this is no longer a wrapper, right? it's something else
mpharrigan
changed the title
XPow/YPow/ZPow rotations gateXPow/YPow/ZPow and factor out CirqGateAsBloqBase
mpharrigan
changed the title
XPow/YPow/ZPow and factor out CirqGateAsBloqBaseXPow/YPow/ZPow and factor out CirqGateAsBloqBase
| class CirqGateAsBloq(CirqGateAsBloqBase): | ||
| gate: cirq.Gate |
|
post-hoc review; would appreciate if you could open a small follow-up pr |
This PR does a bunch of changes/improvements in cirq interop
CirqGateAsBloqBaseabstract class which has all the functionality to auto populate the bloq methods using underlying Cirq gates. There is one abstract method calledcirq_gatewhich can be over-riden by derived classes.CirqGateAsBloqsimply derives fromCirqGateAsBloqBaseand uses the user specifiedself.gateas the correspondingcirq_gate.CirqGateAsBloqBaseclass and specify a custom cirq gate as thecirq_gateproperty. For commonly used gates in Cirq-Core, this inheritance based method for wrapping cirq gates solves the discoverability issue and reduces boilerplace code.rotations.pyfile to wrapRx / Ry / Rz / XPow / YPow / ZPowgates. As you can see, the wrapper code is now much simpler.Register(qubits, shape=(nqubits), bitsize=1)toRegister('q', shape=(nqubits), bitsize=1)ifnqubits > 1orRegister('q', shape=(), bitsize=1)if nqbits = 1. Changingqubitstoqis nicer because we don't need to worry about singular / plural when number of qubits are > 1 or = 1.The
XPow/YPow/ZPowgates have an option to configure the global phase and follow a pattern similar to Cirq. These gates are useful when implementing phase gradient states. Docstrings for these gates are copied over from Cirq.As a follow up, we can also consider making these new gates implementation more robust so we can get rid of
XGate/YGate/ZGatethat also currently exist.