optimize_for_target_gateset returns a non-optimal circuit #6422
Comments
|
+1 There should be some check that it isn't increasing the number of moments. |
NoureldinYosri
added
triage/discuss
Explanation of what's happening in this exampleSo, the idea behind In this specific case, the "group" transformers to run is encapsulated in the
As you can see, this is not the desired behavior for this use case because ideally, we'd like to merge only the moments with single qubit gates and leve alone the two qubit gate moments (that already contain CZs and thus don't need to be decomposed at all). Steps 1 & 2 above are part of "preprocessing" transformers for the
After step-3, all operations should be converted to the target gateset but hopefully with a shorter depth (this is best effort, and clearly not happening here). We then run post-processing transformers to allow obvious cleanups like
Discussion on potential workarounds to fix the behavior
import cirq
from typing import List
class CustomCZTargetGateset(cirq.CZTargetGateset):
@property
def preprocess_transformers(self) -> List['cirq.TRANSFORMER']:
"""List of transformers which should be run before decomposing individual operations."""
return [cirq.merge_single_qubit_moments_to_phxz]
@property
def postprocess_transformers(self) -> List['cirq.TRANSFORMER']:
"""List of transformers which should be run after decomposing individual operations."""
return [
cirq.drop_negligible_operations,
cirq.drop_empty_moments,
]
cirq.optimize_for_target_gateset(circuit=circ,gateset=CustomCZTargetGateset())Results in the following ouptut as expected. This should be the preferred approach because the goal of the entire Cirq transformers infrastructure was to provide more control to users for their specific compilation requirements since it's hard to capture all the corner cases as part of a generic compiler (preserve moment structure vs reduce depth etc.)
@eliottrosenberg @gauravgyawali @NoureldinYosri Hope this provides more context on the intention behind the original design. Let me know your thoughts. |
|
What if we just changed |
|
I think it would be helpful from the perspective of device implementation to be able to optimize the number of moments by calling this function. The optimized moment structure can make a big difference in the quality of data. Perhaps, we can have a keyword that lets you turn off the default moment structure preservation? |
|
I think two parameters should be introduced |
|
Somewhat related -- a simple circuit like this: Returns |
|
@ikd-sci update: I was looking at the diagram rather than the moment. The decomposer should have used CZ rather than CZ**-1 so I will findout why it didn't. |
|
@tanujkhattar @eliottrosenberg @gauravgyawali I updated #6426 to have an argument for preserving the moment structure. The desired result for the circuit in the issue is then obtained by calling the method with Alternatively we can discard the PR and implement a custom gateset as @tanujkhattar suggested. however I do think that at least the |
|
@NoureldinYosri IMO, it's better to add Regarding |
@NoureldinYosri I just tried Ilya's example, and indeed, it produced |
|
@eliottrosenberg @ikd-sci sorry, I was looking at the diagram rather than the moment. The decomposer should have used CZ rather than CZ**-1 so I will findout why it didn't. |
|
looking deeper into why it uses CZ**-1 rather than CZ. it looks like that initially it uses CZ as expected and produces the circuit Z**0.75(q(0))
X**-0.25(q(1))
X**0.5000000000000001(q(0))
Y**-0.5(q(1))
S**-1(q(0))
Y**-0.5(q(0))
CZ(q(0), q(1))
S**-1(q(0))
S**-1(q(1))
Y**0.5(q(0))
Y**0.5(q(1))
Y**0.5(q(0))
X**-0.24999999999999997(q(1))
Z**-0.75(q(0))but because we compress operations PhX(-0.5)**0.5(q(1))
PhX(0.375)(q(0))
T**-1(q(1))
CZ(q(0), q(1))
PhX(-0.75)**0.5(q(1))
PhX(-0.625)(q(0))
Z**-0.75(q(1))on which it calls PhX(-0.5)**0.5(q(1))
T**-1(q(1))
Z(q(1))
CZ**-1.0(q(0), q(1))
PhX(-0.75)**0.5(q(1))
Z**-0.75(q(1))I'm not sure whether this is a bug or a byproduct of the equivalence between CZ and CZ**-1. I filed #6428 to track invistigating that. |
|
@gauravgyawali @ikd-sci @eliottrosenberg >>> gateset = cirq.CZTargetGateset(preserve_moment_structure=False)
>>> print(cirq.optimize_for_target_gateset(circuit=circ, gateset=gateset, max_num_passes=2))
0: ───PhXZ(a=0.5,x=-0.5,z=1)───@───────
│
1: ───PhXZ(a=-1,x=1,z=0)───────@───@───
│
2: ───PhXZ(a=-0.5,x=0.5,z=0)───@───@───
│
3: ───PhXZ(a=-1,x=1,z=0)───────@───@───
│
4: ───PhXZ(a=0.5,x=-0.5,z=1)───@───@───
│
5: ────────────────────────────@───@───
│
6: ───PhXZ(a=-0.5,x=0.5,z=0)───────@───Also the issue in #6422 (comment) is now fixed >>> circuit = cirq.Circuit([
... cirq.CNOT(cirq.LineQubit(0), cirq.LineQubit(1)),
... cirq.CNOT(cirq.LineQubit(1), cirq.LineQubit(2))])
>>> print(circuit)
0: ───@───────
│
1: ───X───@───
│
2: ───────X───
>>> gateset = cirq.CZTargetGateset(allow_partial_czs=False)
>>> circuit2 = cirq.optimize_for_target_gateset(circuit, gateset=gateset)
cuit2.moments)
>>> print(circuit2)
0: ────────────────────────────@────────────────────────────────────────────────────────
│
1: ───PhXZ(a=0.5,x=-0.5,z=0)───@───PhXZ(a=0.5,x=0.5,z=0)────@───────────────────────────
│
2: ────────────────────────────────PhXZ(a=0.5,x=-0.5,z=0)───@───PhXZ(a=0.5,x=0.5,z=0)───
>>> print(circuit2.moments)
[cirq.Moment(
cirq.PhasedXZGate(axis_phase_exponent=0.4999999999999998, x_exponent=-0.5, z_exponent=0.0).on(cirq.LineQubit(1)),
), cirq.Moment(
cirq.CZ(cirq.LineQubit(0), cirq.LineQubit(1)),
), cirq.Moment(
cirq.PhasedXZGate(axis_phase_exponent=0.4999999999999998, x_exponent=0.5, z_exponent=0.0).on(cirq.LineQubit(1)),
cirq.PhasedXZGate(axis_phase_exponent=0.4999999999999998, x_exponent=-0.5, z_exponent=0.0).on(cirq.LineQubit(2)),
), cirq.Moment(
cirq.CZ(cirq.LineQubit(1), cirq.LineQubit(2)),
), cirq.Moment(
cirq.PhasedXZGate(axis_phase_exponent=0.4999999999999998, x_exponent=0.5, z_exponent=0.0).on(cirq.LineQubit(2)),
)] |
|
Thank you, Nour!! |
Using cirq.optimize_for_target_gateset(circuit, gateset=cirq.CZTargetGateset()) returns a circuit with more moments. First, it doesn't combine the multiple single qubit rotations into a single PhasedXZGate. Second, it distributes the operations of an optimal moment to make it non-optimal
How to reproduce the issue
I find that using cirq.merge_single_qubit_moments_to_phxz(circ) first fixes the issue.
Cirq version
1.2.0.dev20230628175157
The text was updated successfully, but these errors were encountered: