Initial draft for convert_to_sqrt_iswap #2528
Conversation
dstrain115
commented
Nov 12, 2019
dstrain115
commented
- Optimizer to decompile to ISWAP ** -0.5 gates.
- Ported from cirq_internal
- Modified to accept symbol input as well.
- Optimizer to decompile to ISWAP ** -0.5 gates. - Ported from cirq_internal - Modified to accept symbol input as well.
…/Cirq-1 into convert_to_sqrt_iswap
| return NotImplemented | ||
|
|
||
| if isinstance(gate, cirq.CZPowGate): | ||
| turns = cast(cirq.CZPowGate, gate).exponent |
There was a problem hiding this comment.
I think mypy understands after an isinstance check that gate is an instance of cirq.CZPowGate so this cast should not be needed. Same thing for the other cases below. Also, consider calling the variable exponent instead of turns.
There was a problem hiding this comment.
Weird. I totally thought mypy complained about this, but I guess not.
| Returns: | ||
| True if the operation is native to the gate set, false otherwise. | ||
| """ | ||
| return (isinstance(op, cirq.GateOperation) and |
There was a problem hiding this comment.
This check is not needed since Operation always has a gate attribute.
| ISwapPowGate or with the equivalent FSimGate. | ||
| """ | ||
| if (isinstance(gate, cirq.FSimGate) and | ||
| not isinstance(gate.theta, sympy.Symbol) and |
There was a problem hiding this comment.
gate.theta could be a more complicated expression than just a symbol. I think this should check sympy.Basic instead, though it's probably safer to use cirq.is_parametrized. Same thing for the ISwapPowGate case below.
| def convert(self, op: cirq.Operation) -> List[cirq.Operation]: | ||
|
|
||
| def on_stuck_raise(bad): | ||
| return TypeError("Don't know how to work with {!r}. " |
There was a problem hiding this comment.
nit: could use f-string instead of .format.
|
|
||
|
|
||
| def cphase_to_sqrt_iswap(a, b, turns): | ||
| """Implement a C-Phase gate using two CZ gates and single-qubit |
There was a problem hiding this comment.
"using two SqrtISwap gates"
| import cirq | ||
| import sympy | ||
|
|
||
| SQRT_ISWAP = cirq.ISWAP**-0.5 |
There was a problem hiding this comment.
Note that the device can handle both cirq.ISWAP**0.5 and cirq.ISWAP**-0.5 natively, since these are related by ISWAP(a, b)**0.5 = Z(a) ISWAP(a, b)**-0.5 Z(b). This latter sequence of an iswap conjugated by Zs on one qubit appears in various places in the decompositions below, so we might be able to simplify things by defining:
SQRT_ISWAP = cirq.ISWAP**0.5
SQRT_ISWAP_INV = cirq.ISWAP**-0.5and then using both of these in the decompositions. This should avoid confusion about why we're choosing the negative exponent for "SQRT_ISWAP", and it also might make some of the decompositions more symmetrical.
| (cirq.ISwapPowGate(exponent=-0.5), 1), | ||
| (cirq.FSimGate(theta=np.pi / 4, phi=0), 1), | ||
| ] + | ||
| [(cirq.SwapPowGate(exponent=a), 13) for a in np.linspace(0, 2 * np.pi, 20)] |
There was a problem hiding this comment.
nit: instead of concatenating lists, might be cleaner to splat iterables into a single list:
@pytest.mark.parametrize(
'gate, expected_length',
[
(cast(cirq.Gate, cirq.ISWAP), 7),
...
(cirq.FSimGate(theta=np.pi / 4, phi=0), 1),
*[(cirq.CZPowGate(exponent=a), 8) for a in np.linspace(0, 2 * np.pi, 20)],
*[(cirq.ISwapPowGate(exponent=a), 5) for a in np.linspace(0, 2 * np.pi, 20)],
...
])I have no idea which of these the formatter will handle better, so feel free to ignore if this doesn't help.
There was a problem hiding this comment.
Good idea. It looks a lot better.
|
|
||
|
|
||
| def test_cphase(): | ||
| """Test if the sqrt_iswap synthesis for a givens rotation is correct""" |
| gate = op.gate | ||
|
|
||
| if len(op.qubits) != 2: | ||
| return NotImplemented |
There was a problem hiding this comment.
nit: after this check you could do something like q0, q1 = op.qubits and then use q0 and q1 to simplify the expressions below.
There was a problem hiding this comment.
Good suggestion, done.
|
|
||
| def convert(self, op: cirq.Operation) -> List[cirq.Operation]: | ||
|
|
||
| def on_stuck_raise(bad): |
There was a problem hiding this comment.
I'm not sure the performance implication of defining a local function like this, but it might be faster to make this an internal method on the class instead so it doesn't get redefined on each call. Might be worth benchmarking to see if that makes any difference when optimizing typical circuits.
There was a problem hiding this comment.
Not sure that it matters, but we don't really need an inline function anyway, so I just moved it out.
