Cirq compiles to unexpected gateset

[jordandsullivan]

Describe the bug
We compile all circuits with several other quantum compilers to benchmark UCC. To make the logging more straightfoward, we defined multiple compile helper functions to wrap the complexity into a unified interface [links to compile functions for Qiskit, Cirq, PyTKET)

Although by default, all the compile functions used in our UCC benchmarks should target the same gateset for proper comparison, @natestemen has observed the cirq_compile function compiling to a different gateset -- including the PhasedXZ and CZPow gates.

To Reproduce
Steps to reproduce the behavior:
@natestemen please add your repro script/snippet in this issue

Expected behavior
The cirq_compiled circuit should return {Cirq.rx, Cirq.rx, Cirq.rz, Cirq.cx,, OpType.H}),

Additional context
My guess is that the target gateset selected here is incorrect,

ucc/benchmarks/scripts/common.py

Lines 104 to 105 in d6abe58

	def cirq_compile(cirq_circuit):
	return optimize_for_target_gateset(cirq_circuit, gateset=CZTargetGateset())

and we may need to

1. Define a custom gateset using the gates defined in their docs here (single qubit) and here (two-qubit)
2. Figure out the proper constructor to constitute a valid gateset accepted by cirq.optimize_for_target_gateset.

[natestemen]

# benchmarks/scripts/test_gateset.py

from common import cirq_compile, qiskit_compile, get_native_rep

qasm_path = "../qasm_circuits/qasm2/benchpress/qaoa_barabasi_albert_N10_3reps_basis_rz_rx_ry_cx.qasm"

with open(qasm_path) as f:
    qasm_str = f.read()

cirq_rep = get_native_rep(qasm_str, "cirq")
qiskit_rep = get_native_rep(qasm_str, "qiskit")

# CIRQ
print("cirq pre-compiled",  set(type(op.gate).__name__ for op in cirq_rep.all_operations()))
print("cirq post-compiled", set(type(op.gate).__name__ for op in cirq_compile(cirq_rep).all_operations()))

# QISKIT
print("qiskit pre-compiled",  set(qiskit_rep.count_ops().keys()))
print("qiskit post-compiled", set(qiskit_compile(qiskit_rep).count_ops().keys()))

2. Figure out the proper constructor to constitute a valid gateset accepted by cirq.optimize_for_target_gateset.

This looks nontrivial from the optimize_for_target_gateset documentation since a user has to specify a CompilationTargetGateset object which instead of just being a list of gates is an ABC where one has to define a method decompose_to_target_gateset. :/

[jordandsullivan]

So I tried just eliminating the gateset argument and got this back, which I believe is equivalent to the gateset we need, just represents the rotation as an exponential:

I manually triggered a re-run of the benchmarking workflow here, so we will see what it does to the results: https://github.com/unitaryfund/ucc/actions/runs/13016680357

[jordandsullivan]

Okay that definitely isn't the right move, because if you take away the target gateset, cirq appears to just not do anything to the circuits:

376f647#diff-b2990d4889b9c2640e4b91506d546a9b721e63dbc2b50dc1a4565c19302f1170

[jordandsullivan]

Reverted to use the CZ target gateset in the dev branch

[bachase]

Following the discussion above, I agree with @natestemen that

This looks nontrivial from the optimize_for_target_gateset documentation since a user has to specify a CompilationTargetGateset object which instead of just being a list of gates is an ABC where one has to define a method decompose_to_target_gateset. :/

Digging a bit deeper, cirq does require users to implement an instance of the abstract base class that does all the heavy lifting. They do have TwoQubitCompilationTargetGateset which implements some transformation logic. But it requires the derived class to "provide analytical decomposition of any 2q unitary using gates from the target gateset". It also looks like the provided decomposition of single qubit operations relies on a phase XZ gate (cirq.PhasedXZGate).

Looking across the cirq codebase, there are the following classes that implement TwoQubitCompilationTargetGateset

Class Name	Single Qubit Gates	Two Qubit Gates
AQTTargetGateset	cirq.ZPowGate, cirq.PhasedXPowGate	cirq.XXPowGate
CZTargetGateset	cirq.PhasedXZGate	cirq.CZ / cirq.CZPowGate
SqrtISwapTargetGateset	cirq.PhasedXZGate	cirq.SQRT_ISWAP / cirq.SQRT_ISWAP_INV
CliffordTargetGateset	cirq.SingleQubitCliffordGate and/or cirq.PauliStringPhasorGate	cirq.CZ
SycamoreTargetGateset	cirq.XPowGate, cirq.YPowGate, cirq.ZPowGate, cirq.PhasedXZGate, cirq.PhasedXPowGate	cirq.SYC
IonQTargetGateset	cirq.XPowGate, cirq.YPowGate, cirq.ZPowGate, cirq.H	cirq.XXPowGate, cirq.YYPowGate, cirq.ZZPowGate
IonQArbitraryTargetGateset	cirq.XPowGate, cirq.YPowGate, cirq.ZPowGate, cirq.H	cirq.XXPowGate, cirq.YYPowGate, cirq.ZZPowGate, cirq.CNOT, cirq.SWAP

It looks like IonQ is the closest to the RX,RY,RZ,H and CX target gets, although the *Pow gate have an extra global phase (not sure why). None of these use CX (which I think would be cirq.CNOT).

If we want an apples to apples gateset comparison, I think we'd need to implement this ourselves, and could start from the IonQTargets as examples. This will of course add performance implications since we aren't using a cirq provided implementation, but its a start.

I'll start looking into those steps.

[bachase]

Status update -- I tested with IonQTargetGateset and it seems to work (noting that for cirq, CNOT is actually a special case of CXPowGate. However, the source circuit I tested on only had those gates already, so I also need to test some other cases to see how it transpiles them.