Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add 2q fractional gates to the UnitarySynthesis tranpiler pass #13568

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions crates/accelerate/src/two_qubit_decompose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2448,6 +2448,7 @@ impl RXXEquivalent {
#[pyclass(module = "qiskit._accelerate.two_qubit_decompose", subclass)]
pub struct TwoQubitControlledUDecomposer {
rxx_equivalent_gate: RXXEquivalent,
euler_basis: EulerBasis,
#[pyo3(get)]
scale: f64,
}
Expand Down Expand Up @@ -2544,9 +2545,8 @@ impl TwoQubitControlledUDecomposer {
let decomposer_inv =
TwoQubitWeylDecomposition::new_inner(mat.view(), Some(DEFAULT_FIDELITY), None)?;

let euler_basis = EulerBasis::ZYZ;
let mut target_1q_basis_list = EulerBasisSet::new();
target_1q_basis_list.add_basis(euler_basis);
target_1q_basis_list.add_basis(self.euler_basis);

// Express the RXXGate in terms of the user-provided RXXGate equivalent gate.
let mut gates = Vec::with_capacity(13);
Expand Down Expand Up @@ -2583,14 +2583,14 @@ impl TwoQubitControlledUDecomposer {
gates.push((None, smallvec![self.scale * angle], smallvec![0, 1]));

if let Some(unitary_k1r) = unitary_k1r {
global_phase += unitary_k1r.global_phase;
global_phase -= unitary_k1r.global_phase;
for gate in unitary_k1r.gates.into_iter().rev() {
let (inv_gate_name, inv_gate_params) = invert_1q_gate(gate);
gates.push((Some(inv_gate_name), inv_gate_params, smallvec![0]));
}
}
if let Some(unitary_k1l) = unitary_k1l {
global_phase += unitary_k1l.global_phase;
global_phase -= unitary_k1l.global_phase;
for gate in unitary_k1l.gates.into_iter().rev() {
let (inv_gate_name, inv_gate_params) = invert_1q_gate(gate);
gates.push((Some(inv_gate_name), inv_gate_params, smallvec![1]));
Expand Down Expand Up @@ -2687,9 +2687,8 @@ impl TwoQubitControlledUDecomposer {
let target_decomposed =
TwoQubitWeylDecomposition::new_inner(unitary, Some(DEFAULT_FIDELITY), None)?;

let euler_basis = EulerBasis::ZYZ;
let mut target_1q_basis_list = EulerBasisSet::new();
target_1q_basis_list.add_basis(euler_basis);
target_1q_basis_list.add_basis(self.euler_basis);

let c1r = target_decomposed.K1r.view();
let c2r = target_decomposed.K2r.view();
Expand Down Expand Up @@ -2728,13 +2727,13 @@ impl TwoQubitControlledUDecomposer {
global_phase += gates1.global_phase;

if let Some(unitary_c1r) = unitary_c1r {
global_phase -= unitary_c1r.global_phase;
global_phase += unitary_c1r.global_phase;
for gate in unitary_c1r.gates.into_iter() {
gates1.gates.push((Some(gate.0), gate.1, smallvec![0]));
}
}
if let Some(unitary_c1l) = unitary_c1l {
global_phase -= unitary_c1l.global_phase;
global_phase += unitary_c1l.global_phase;
for gate in unitary_c1l.gates.into_iter() {
gates1.gates.push((Some(gate.0), gate.1, smallvec![1]));
}
Expand All @@ -2751,11 +2750,17 @@ impl TwoQubitControlledUDecomposer {
/// Args:
/// rxx_equivalent_gate: Gate that is locally equivalent to an :class:`.RXXGate`:
/// :math:`U \sim U_d(\alpha, 0, 0) \sim \text{Ctrl-U}` gate.
/// euler_basis: Basis string to be provided to :class:`.OneQubitEulerDecomposer`
/// for 1Q synthesis.
/// Raises:
/// QiskitError: If the gate is not locally equivalent to an :class:`.RXXGate`.
#[new]
#[pyo3(signature=(rxx_equivalent_gate))]
pub fn new(py: Python, rxx_equivalent_gate: RXXEquivalent) -> PyResult<Self> {
#[pyo3(signature=(rxx_equivalent_gate, euler_basis="ZYZ"))]
pub fn new(
py: Python,
rxx_equivalent_gate: RXXEquivalent,
euler_basis: &str,
) -> PyResult<Self> {
let atol = DEFAULT_ATOL;
let test_angles = [0.2, 0.3, PI2];

Expand Down Expand Up @@ -2819,6 +2824,7 @@ impl TwoQubitControlledUDecomposer {
Ok(TwoQubitControlledUDecomposer {
scale,
rxx_equivalent_gate,
euler_basis: EulerBasis::__new__(euler_basis)?,
})
}

Expand Down
4 changes: 3 additions & 1 deletion qiskit/synthesis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
TwoQubitBasisDecomposer
XXDecomposer
TwoQubitWeylDecomposition
TwoQubitControlledUDecomposer

.. autofunction:: two_qubit_cnot_decompose

Expand Down Expand Up @@ -147,7 +148,7 @@
Multipliers
-----------

.. autofunction:: multiplier_cumulative_h18
.. autofunction:: multiplier_cumulative_h18
.. autofunction:: multiplier_qft_r17

"""
Expand Down Expand Up @@ -200,6 +201,7 @@
TwoQubitBasisDecomposer,
two_qubit_cnot_decompose,
TwoQubitWeylDecomposition,
TwoQubitControlledUDecomposer,
)
from .multi_controlled import (
synth_mcmt_vchain,
Expand Down
24 changes: 19 additions & 5 deletions qiskit/synthesis/two_qubit/two_qubit_decompose.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,32 +270,46 @@ class TwoQubitControlledUDecomposer:
:math:`U \sim U_d(\alpha, 0, 0) \sim \text{Ctrl-U}`
gate that is locally equivalent to an :class:`.RXXGate`."""

def __init__(self, rxx_equivalent_gate: Type[Gate]):
def __init__(self, rxx_equivalent_gate: Type[Gate], euler_basis: str = "ZYZ"):
r"""Initialize the KAK decomposition.

Args:
rxx_equivalent_gate: Gate that is locally equivalent to an :class:`.RXXGate`:
:math:`U \sim U_d(\alpha, 0, 0) \sim \text{Ctrl-U}` gate.
:math:`U \sim U_d(\alpha, 0, 0) \sim \text{Ctrl-U}` gate.
Valid options are [:class:`.RZZGate`, :class:`.RXXGate`, :class:`.RYYGate`,
:class:`.RZXGate`, :class:`.CPhaseGate`, :class:`.CRXGate`, :class:`.CRYGate`,
:class:`.CRZGate`].
euler_basis: Basis string to be provided to :class:`.OneQubitEulerDecomposer`
for 1Q synthesis.
Valid options are [``'ZYZ'``, ``'ZXZ'``, ``'XYX'``, ``'XZX'``, ``'U'``, ``'U3'``,
``'U321'``, ``'U1X'``, ``'PSX'``, ``'ZSX'``, ``'ZSXX'``, ``'RR'``].

Raises:
QiskitError: If the gate is not locally equivalent to an :class:`.RXXGate`.
"""
if rxx_equivalent_gate._standard_gate is not None:
self._inner_decomposition = two_qubit_decompose.TwoQubitControlledUDecomposer(
rxx_equivalent_gate._standard_gate
rxx_equivalent_gate._standard_gate, euler_basis
)
else:
self._inner_decomposition = two_qubit_decompose.TwoQubitControlledUDecomposer(
rxx_equivalent_gate
rxx_equivalent_gate, euler_basis
)
self.rxx_equivalent_gate = rxx_equivalent_gate
self.scale = self._inner_decomposition.scale
self.euler_basis = euler_basis

def __call__(self, unitary: Operator | np.ndarray, *, atol=DEFAULT_ATOL) -> QuantumCircuit:
def __call__(
self, unitary: Operator | np.ndarray, approximate=False, use_dag=False, *, atol=DEFAULT_ATOL
) -> QuantumCircuit:
"""Returns the Weyl decomposition in circuit form.

Args:
unitary (Operator or ndarray): :math:`4 \times 4` unitary to synthesize.

Returns:
QuantumCircuit: Synthesized quantum circuit.

Note: atol is passed to OneQubitEulerDecomposer.
"""
circ_data = self._inner_decomposition(np.asarray(unitary, dtype=complex), atol)
Expand Down
56 changes: 43 additions & 13 deletions qiskit/transpiler/passes/synthesis/unitary_synthesis.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
RXXGate,
RZXGate,
RZZGate,
RYYGate,
ECRGate,
RXGate,
SXGate,
Expand All @@ -50,6 +51,10 @@
U3Gate,
RYGate,
RGate,
CRXGate,
CRYGate,
CRZGate,
CPhaseGate,
)
from qiskit.converters import circuit_to_dag, dag_to_circuit
from qiskit.dagcircuit.dagcircuit import DAGCircuit
Expand All @@ -62,6 +67,7 @@
from qiskit.synthesis.two_qubit.two_qubit_decompose import (
TwoQubitBasisDecomposer,
TwoQubitWeylDecomposition,
TwoQubitControlledUDecomposer,
)
from qiskit.transpiler.basepasses import TransformationPass
from qiskit.transpiler.coupling import CouplingMap
Expand All @@ -88,16 +94,32 @@
"u3": U3Gate._standard_gate,
"ry": RYGate._standard_gate,
"r": RGate._standard_gate,
"rzz": RZZGate._standard_gate,
"ryy": RYYGate._standard_gate,
"rxx": RXXGate._standard_gate,
"rzx": RXXGate._standard_gate,
"cp": CPhaseGate._standard_gate,
"crx": RXXGate._standard_gate,
"cry": RXXGate._standard_gate,
"crz": RXXGate._standard_gate,
}

KAK_GATE_PARAM_NAMES = {
"rxx": RXXGate,
"rzz": RZZGate,
"ryy": RYYGate,
"rzx": RZXGate,
"cphase": CPhaseGate,
"crx": CRXGate,
"cry": CRYGate,
"crz": CRZGate,
}

KAK_GATE_NAMES = {
"cx": CXGate(),
"cz": CZGate(),
"iswap": iSwapGate(),
"rxx": RXXGate(pi / 2),
"ecr": ECRGate(),
"rzx": RZXGate(pi / 4), # typically pi/6 is also available
}

GateNameToGate = get_standard_gate_name_mapping()
Expand All @@ -106,9 +128,14 @@
def _choose_kak_gate(basis_gates):
"""Choose the first available 2q gate to use in the KAK decomposition."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: It seems we are not choosing the element according to some order, but using set pop, which is random, leading to nondeterministic behavior. Can this be avoided (or already avoided and I'm missing something)? e.g.

kak_gate = sorted([KAK_GATE_PARAM_NAMES[kak_gate_param] for kak_gate_params in kak_gates_params])[0]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point @gadial, I had always assumed that this randomness was a reflection of the fact that there can be several equally valid gates. And if that's case, I wonder if we should favor the one that comes first in alphabetical order? At the same time, I am not sure how conscious some of these decisions were. There are other parts of the code (the parts that were migrated to Rust) where we actually rely on insertion order when there are several valid decompositions, so at that point I guess it's as arbitrary as sorting alphabetically. It might be also worth noting that these cases don't come up often in real scenarios because we rarely see a basis set with more than one 2q gate (although it can definitely happen).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. I'll look into it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ElePT. My approach is usually to prefer the arbitrary to the nondeterminisitic, since nondeterministic behavior makes testing more difficult (since it's harder to reproduce problems) but also less resilient (since we are only testing the conditions generated nondeterminisically on the current testing machines, whereas a future version of Python might behave differently).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @gadial's comment. If the user provides several 2q KAK gates, this "pop" just randomly chooses one of them. We would probably want a more deterministic behavior (perhaps favoring more "commonly" used 2q gates).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that sounds reasonable, it helps with benchmarking and bug-catching.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in eeff4cd

kak_gate = None
kak_gates = set(basis_gates or []).intersection(KAK_GATE_NAMES.keys())
if kak_gates:
kak_gate = KAK_GATE_NAMES[kak_gates.pop()]
kak_gates = sorted(set(basis_gates or []).intersection(KAK_GATE_NAMES.keys()))
kak_gates_params = sorted(set(basis_gates or []).intersection(KAK_GATE_PARAM_NAMES.keys()))

if kak_gates_params:
kak_gate = KAK_GATE_PARAM_NAMES[kak_gates_params[0]]

elif kak_gates:
kak_gate = KAK_GATE_NAMES[kak_gates[0]]

return kak_gate

Expand Down Expand Up @@ -151,14 +178,17 @@ def _decomposer_2q_from_basis_gates(basis_gates, pulse_optimize=None, approximat
kak_gate = _choose_kak_gate(basis_gates)
euler_basis = _choose_euler_basis(basis_gates)
basis_fidelity = approximation_degree or 1.0
if isinstance(kak_gate, RZXGate):
backup_optimizer = TwoQubitBasisDecomposer(
CXGate(),
basis_fidelity=basis_fidelity,
euler_basis=euler_basis,
pulse_optimize=pulse_optimize,
)
decomposer2q = XXDecomposer(euler_basis=euler_basis, backup_optimizer=backup_optimizer)
# if isinstance(kak_gate, RZXGate):
# backup_optimizer = TwoQubitBasisDecomposer(
# CXGate(),
# basis_fidelity=basis_fidelity,
# euler_basis=euler_basis,
# pulse_optimize=pulse_optimize,
# )
# decomposer2q = XXDecomposer(euler_basis=euler_basis, backup_optimizer=backup_optimizer)

if kak_gate in KAK_GATE_PARAM_NAMES.values():
decomposer2q = TwoQubitControlledUDecomposer(kak_gate, euler_basis)
elif kak_gate is not None:
decomposer2q = TwoQubitBasisDecomposer(
kak_gate,
Expand Down
2 changes: 1 addition & 1 deletion test/python/compiler/test_transpiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ def test_block_collection_reduces_1q_gate(self, basis_gates, gate_counts):
basis_gates=[
["u3", "cx"],
["rx", "rz", "iswap"],
["rx", "ry", "rxx"],
["ry", "rz", "rxx"],
],
)
def test_translation_method_synthesis(self, optimization_level, basis_gates):
Expand Down
30 changes: 24 additions & 6 deletions test/python/synthesis/test_synthesis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1426,14 +1426,32 @@ def test_approx_supercontrolled_decompose_phase_3_use_random(self, seed, delta=0
class TestTwoQubitControlledUDecompose(CheckDecompositions):
"""Test TwoQubitControlledUDecomposer() for exact decompositions and raised exceptions"""

@combine(seed=range(10), name="seed_{seed}")
def test_correct_unitary(self, seed):
@combine(
seed=range(5),
gate=[RXXGate, RYYGate, RZZGate, RZXGate, CPhaseGate, CRZGate, CRXGate, CRYGate],
euler_basis=[
"ZYZ",
"ZXZ",
"XYX",
"XZX",
"RR",
"U",
"U3",
"U321",
"PSX",
"ZSX",
"ZSXX",
"U1X",
],
name="seed_{seed}",
)
def test_correct_unitary(self, seed, gate, euler_basis):
"""Verify unitary for different gates in the decomposition"""
unitary = random_unitary(4, seed=seed)
for gate in [RXXGate, RYYGate, RZZGate, RZXGate, CPhaseGate, CRZGate, CRXGate, CRYGate]:
decomposer = TwoQubitControlledUDecomposer(gate)
circ = decomposer(unitary)
self.assertEqual(Operator(unitary), Operator(circ))

decomposer = TwoQubitControlledUDecomposer(gate, euler_basis)
circ = decomposer(unitary)
self.assertEqual(Operator(unitary), Operator(circ))

def test_not_rxx_equivalent(self):
"""Test that an exception is raised if the gate is not equivalent to an RXXGate"""
Expand Down
3 changes: 2 additions & 1 deletion test/python/transpiler/test_unitary_synthesis.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ def test_empty_basis_gates(self):
@data(
["u3", "cx"],
["u1", "u2", "u3", "cx"],
["rx", "ry", "rxx"],
["ry", "rz", "rxx"],
["rx", "rz", "rzz"],
["rx", "rz", "iswap"],
["u3", "rx", "rz", "cz", "iswap"],
)
Expand Down
Loading