Fix handling of multiple available target ops in UnitarySynthesis (#7795

)

* Fix handling of multiple available target ops in UnitarySynthesis

We recently added support for working natively with a Target to the
UnitarySynthesis pass in #7775. However, in that PR we took a naive
approach to dealing with the KAK gate for decomposition which would
result in the pass not correctly generating an output circuit.
Previously, it would just pick the first KAK gate that was present in
the target whether or not it was the best choice available. There was
also a bug in #7775 where it would only look at the cx error rate
instead of the error rate for the selected kak gate. This bug could
result in cases of the output of the unitary synthesis would be
considered invalid by later passes because it didn't properly respect
the constraints of the backend. This commit fixes these issues by
instead considering all combinations of 1q and kak gate in the target
and picking the combination with the lowest error rate available on the
selected qubits. Then the unit testing for the pass is expanded to cover
the use of the target to ensure we're testing the full path with a
target specified.

* Fix test assertions

The test assertions in the new tests were previously flawed and would
evaluate as True if there were no gates for the in the circuit we
were checking. This commit fixes the tests so we're actually
asserting things correctly.

* Reorganize helper function split

This commit reorganizes the internal helper functions to split the
target and non-target paths to be completely separate. Then the target
path adds a cache for the 2q decomposer to use for the specified qubits
so we don't have to do the lookup more than once on subsequent runs.

* Fix reverse decomposer selection and add a test

* Improve test assertions

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Update test with new fake mumbai backend name

* Update docstring to explain basis_gates or target are required

Co-authored-by: Jake Lishman <jake@binhbar.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/transpiler/passes/synthesis/unitary_synthesis.py

@@ -15,6 +15,7 @@
 from math import pi, inf
 from typing import List, Union
 from copy import deepcopy
+from itertools import product
 
 from qiskit.converters import circuit_to_dag
 from qiskit.transpiler import CouplingMap, Target
@@ -25,6 +26,7 @@
 from qiskit.quantum_info.synthesis import one_qubit_decompose
 from qiskit.quantum_info.synthesis.xx_decompose import XXDecomposer
 from qiskit.quantum_info.synthesis.two_qubit_decompose import TwoQubitBasisDecomposer
+from qiskit.circuit.parameter import Parameter
 from qiskit.circuit.library.standard_gates import (
     iSwapGate,
     CXGate,
@@ -37,28 +39,47 @@
 from qiskit.providers.models import BackendProperties
 
 
-def _choose_kak_gate(basis_gates):
-    """Choose the first available 2q gate to use in the KAK decomposition."""
+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
+}
 
-    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
-    }
 
+def _choose_kak_gate(basis_gates):
+    """Choose the first available 2q gate to use in the KAK decomposition."""
     kak_gate = None
-    kak_gates = set(basis_gates or []).intersection(kak_gate_names.keys())
+    kak_gates = set(basis_gates or []).intersection(KAK_GATE_NAMES.keys())
     if kak_gates:
-        kak_gate = kak_gate_names[kak_gates.pop()]
+        kak_gate = KAK_GATE_NAMES[kak_gates.pop()]
 
     return kak_gate
 
 
+def _find_matching_kak_gates(target):
+    """Return list of available 2q gates to use in the KAK decomposition."""
+    kak_gates = []
+    for name in target:
+        if name in KAK_GATE_NAMES:
+            kak_gates.append(KAK_GATE_NAMES[name])
+            continue
+        op = target.operation_from_name(name)
+        if isinstance(op, RXXGate) and (
+            isinstance(op.params[0], Parameter) or op.params[0] == pi / 2
+        ):
+            kak_gates.append((KAK_GATE_NAMES["rxx"], name))
+        elif isinstance(op, RZXGate) and (
+            isinstance(op.params[0], Parameter) or op.params[0] == pi / 4
+        ):
+            kak_gates.append((KAK_GATE_NAMES["rzx"], name))
+    return kak_gates
+
+
 def _choose_euler_basis(basis_gates):
-    """ "Choose the first available 1q basis to use in the Euler decomposition."""
+    """Choose the first available 1q basis to use in the Euler decomposition."""
     basis_set = set(basis_gates or [])
 
     for basis, gates in one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES.items():
@@ -69,6 +90,16 @@ def _choose_euler_basis(basis_gates):
     return None
 
 
+def _find_matching_euler_bases(target):
+    """Find matching availablee 1q basis to use in the Euler decomposition."""
+    euler_basis_gates = []
+    basis_set = target.keys()
+    for basis, gates in one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES.items():
+        if set(gates).issubset(basis_set):
+            euler_basis_gates.append(basis)
+    return euler_basis_gates
+
+
 def _choose_bases(basis_gates, basis_dict=None):
     """Find the matching basis string keys from the list of basis gates from the backend."""
     if basis_gates is None:
@@ -109,7 +140,7 @@ class UnitarySynthesis(TransformationPass):
 
     def __init__(
         self,
-        basis_gates: List[str],
+        basis_gates: List[str] = None,
         approximation_degree: float = 1,
         coupling_map: CouplingMap = None,
         backend_props: BackendProperties = None,
@@ -129,7 +160,10 @@ def __init__(
         exactly.
 
         Args:
-            basis_gates (list[str]): List of gate names to target.
+            basis_gates (list[str]): List of gate names to target. If this is
+                not specified the ``target`` argument must be used. If both this
+                and the ``target`` are specified the value of ``target`` will
+                be used and this will be ignored.
             approximation_degree (float): Closeness of approximation
                 (0: lowest, 1: highest).
             coupling_map (CouplingMap): the coupling map of the backend
@@ -169,7 +203,7 @@ def __init__(
                 plugin has no extra arguments. Refer to the documentation of
                 your unitary synthesis plugin on how to use this.
             target: The optional :class:`~.Target` for the target device the pass
-                is compiling for. IF specified this will supersede the values
+                is compiling for. If specified this will supersede the values
                 set for ``basis_gates``, ``coupling_map``, and ``backend_props``.
         """
         super().__init__()
@@ -377,6 +411,82 @@ def supported_bases(self):
     def supports_target(self):
         return True
 
+    def __init__(self):
+        super().__init__()
+        self._decomposer_cache = {}
+
+    def _find_decomposer_2q_from_target(self, target, qubits, pulse_optimize):
+        qubits_tuple = tuple(qubits)
+        reverse_tuple = (qubits[1], qubits[0])
+        if qubits_tuple in self._decomposer_cache:
+            return self._decomposer_cache[qubits_tuple]
+
+        matching = {}
+        reverse = {}
+        kak_gates = _find_matching_kak_gates(target)
+        euler_basis_gates = _find_matching_euler_bases(target)
+        decomposers_2q = []
+        # find all decomposers
+        for kak_gate, euler_basis in product(kak_gates, euler_basis_gates):
+            gate_name = None
+            if isinstance(kak_gate, tuple):
+                gate_name = kak_gate[1]
+                kak_gate = kak_gate[0]
+            if isinstance(kak_gate, RZXGate):
+                backup_optimizer = TwoQubitBasisDecomposer(
+                    CXGate(), euler_basis=euler_basis, pulse_optimize=pulse_optimize
+                )
+                decomposer = XXDecomposer(
+                    euler_basis=euler_basis, backup_optimizer=backup_optimizer
+                )
+                if gate_name is not None:
+                    decomposer.gate_name = gate_name
+                decomposers_2q.append(decomposer)
+            elif kak_gate is not None:
+                decomposer = TwoQubitBasisDecomposer(
+                    kak_gate, euler_basis=euler_basis, pulse_optimize=pulse_optimize
+                )
+                if gate_name is not None:
+                    decomposer.gate_name = gate_name
+                decomposers_2q.append(decomposer)
+
+        # Find lowest error matching or reverse decomposer and use that
+        for index, decomposer in enumerate(decomposers_2q):
+            gate_name = getattr(decomposer, "gate_name", decomposer.gate.name)
+            props_dict = target[gate_name]
+            if target.instruction_supported(gate_name, qubits_tuple):
+                if props_dict is None or None in props_dict:
+                    error = 0.0
+                else:
+                    error = getattr(props_dict[qubits_tuple], "error", 0.0)
+                    if error is None:
+                        error = 0.0
+                matching[index] = error
+            # Skip reverse check if we already have matching
+            elif not matching and target.instruction_supported(gate_name, reverse_tuple):
+                if props_dict is None or None in props_dict:
+                    error = 0.0
+                else:
+                    error = getattr(props_dict[reverse_tuple], "error", 0.0)
+                    if error is None:
+                        error = 0.0
+                reverse[index] = error
+        preferred_direction = None
+        if matching:
+            preferred_direction = [0, 1]
+            min_error_index = min(matching, key=matching.get)
+            decomposer2q = decomposers_2q[min_error_index]
+        elif reverse:
+            preferred_direction = [1, 0]
+            min_error_index = min(reverse, key=reverse.get)
+            decomposer2q = decomposers_2q[min_error_index]
+        # If no matching or reverse direction is found just pick one, if natural direction is
+        # enforced it will fail later
+        else:
+            decomposer2q = decomposers_2q[0]
+        self._decomposer_cache[qubits_tuple] = (decomposer2q, preferred_direction)
+        return (decomposer2q, preferred_direction)
+
     def run(self, unitary, **options):
         # Approximation degree is set directly as an attribute on the
         # instance by the UnitarySynthesis pass here as it's not part of
@@ -398,7 +508,13 @@ def run(self, unitary, **options):
         else:
             decomposer1q = None
 
-        decomposer2q = _basis_gates_to_decomposer_2q(basis_gates, pulse_optimize=pulse_optimize)
+        preferred_direction = None
+        if target is not None:
+            decomposer2q, preferred_direction = self._find_decomposer_2q_from_target(
+                target, qubits, pulse_optimize
+            )
+        else:
+            decomposer2q = _basis_gates_to_decomposer_2q(basis_gates, pulse_optimize=pulse_optimize)
 
         synth_dag = None
         wires = None
@@ -407,7 +523,7 @@ def run(self, unitary, **options):
                 return None
             synth_dag = circuit_to_dag(decomposer1q._decompose(unitary))
         elif unitary.shape == (4, 4):
-            if decomposer2q is None:
+            if not decomposer2q:
                 return None
             synth_dag, wires = self._synth_natural_direction(
                 unitary,
@@ -420,6 +536,7 @@ def run(self, unitary, **options):
                 approximation_degree,
                 pulse_optimize,
                 target,
+                preferred_direction,
             )
         else:
             synth_dag = circuit_to_dag(isometry.Isometry(unitary, 0, 0).definition)
@@ -438,16 +555,15 @@ def _synth_natural_direction(
         approximation_degree,
         pulse_optimize,
         target,
+        preferred_direction=None,
     ):
-        preferred_direction = None
         synth_direction = None
         physical_gate_fidelity = None
         wires = None
-        if natural_direction in {None, True} and (coupling_map or target):
-            if target is not None:
-                cmap = target.build_coupling_map(two_q_gate=decomposer2q.gate.name)
-            else:
-                cmap = coupling_map
+        if natural_direction in {None, True} and (
+            coupling_map or (target is not None and decomposer2q and not preferred_direction)
+        ):
+            cmap = coupling_map
             neighbors0 = cmap.neighbors(qubits[0])
             zero_one = qubits[1] in neighbors0
             neighbors1 = cmap.neighbors(qubits[1])
@@ -459,11 +575,12 @@ def _synth_natural_direction(
         if (
             natural_direction in {None, True}
             and preferred_direction is None
-            and ((gate_lengths and gate_errors) or target)
+            and (gate_lengths and gate_errors)
         ):
             len_0_1 = inf
             len_1_0 = inf
-            twoq_gate_lengths = gate_lengths.get(decomposer2q.gate.name)
+            gate_name = getattr(decomposer2q, "gate_name", decomposer2q.gate.name)
+            twoq_gate_lengths = gate_lengths.get(gate_name)
             if twoq_gate_lengths:
                 len_0_1 = twoq_gate_lengths.get((qubits[0], qubits[1]), inf)
                 len_1_0 = twoq_gate_lengths.get((qubits[1], qubits[0]), inf)
@@ -472,7 +589,7 @@ def _synth_natural_direction(
             elif len_1_0 < len_0_1:
                 preferred_direction = [1, 0]
             if preferred_direction:
-                twoq_gate_errors = gate_errors.get("cx")
+                twoq_gate_errors = gate_errors.get(gate_name)
                 gate_error = twoq_gate_errors.get(
                     (qubits[preferred_direction[0]], qubits[preferred_direction[1]])
                 )
@@ -488,7 +605,10 @@ def _synth_natural_direction(
             basis_fidelity = approximation_degree
         else:
             basis_fidelity = physical_gate_fidelity
-        synth_circ = decomposer2q(su4_mat, basis_fidelity=basis_fidelity)
+        if not isinstance(decomposer2q, XXDecomposer):
+            synth_circ = decomposer2q(su4_mat, basis_fidelity=basis_fidelity)
+        else:
+            synth_circ = decomposer2q(su4_mat)
         synth_dag = circuit_to_dag(synth_circ)
 
         # if a natural direction exists but the synthesis is in the opposite direction,