Update Optimize1qGatesDecomposition to be Target aware

qiskit/transpiler/passes/optimization/optimize_1q_commutation.py

@@ -157,7 +157,7 @@ def _resynthesize(self, new_run):
         NOTE: Returns None when resynthesis is not possible.
         """
         if len(new_run) == 0:
-            return (), QuantumCircuit(1)
+            return QuantumCircuit(1)
 
         return self._optimize1q._resynthesize_run(new_run)
 
@@ -187,6 +187,7 @@ def _step(self, dag):
         runs = dag.collect_1q_runs()
         did_work = False
 
+        qubit_indices = {bit: index for index, bit in enumerate(dag.qubits)}
         for run in runs:
             # identify the preceding blocking gates
             run_clone = copy(run)
@@ -210,13 +211,9 @@ def _step(self, dag):
                 )
 
             # re-synthesize
-            new_preceding_basis, new_preceding_run = self._resynthesize(
-                preceding_run + commuted_preceding
-            )
-            new_succeeding_basis, new_succeeding_run = self._resynthesize(
-                commuted_succeeding + succeeding_run
-            )
-            new_basis, new_run = self._resynthesize(run_clone)
+            new_preceding_run = self._resynthesize(preceding_run + commuted_preceding)
+            new_succeeding_run = self._resynthesize(commuted_succeeding + succeeding_run)
+            new_run = self._resynthesize(run_clone)
 
             # perform the replacement if it was indeed a good idea
             if self._optimize1q._substitution_checks(
@@ -227,7 +224,8 @@ def _step(self, dag):
                     + (new_run or QuantumCircuit(1)).data
                     + (new_succeeding_run or QuantumCircuit(1)).data
                 ),
-                new_basis + new_preceding_basis + new_succeeding_basis,
+                self._optimize1q._basis_gates,
+                qubit_indices[run[0].qargs[0]],
             ):
                 if preceding_run and new_preceding_run is not None:
                     self._replace_subdag(dag, preceding_run, new_preceding_run)

qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py

@@ -12,12 +12,10 @@
 
 """Optimize chains of single-qubit gates using Euler 1q decomposer"""
 
-import copy
 import logging
-
+from functools import partial
 import numpy as np
 
-from qiskit.circuit.library.standard_gates import U3Gate
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.passes.utils import control_flow
 from qiskit.quantum_info.synthesis import one_qubit_decompose
@@ -27,117 +25,93 @@
 
 
 class Optimize1qGatesDecomposition(TransformationPass):
-    """Optimize chains of single-qubit gates by combining them into a single gate."""
+    """Optimize chains of single-qubit gates by combining them into a single gate.
+
+    The decision to replace the original chain with a new resynthesis depends on:
+     - whether the original chain was out of basis: replace
+     - whether the original chain was in basis but resynthesis is lower error: replace
+     - whether the original chain contains a pulse gate: do not replace
+     - whether the original chain amounts to identity: replace with null
+
+     Error is computed as a multiplication of the errors of individual gates on that qubit.
+    """
 
-    def __init__(self, basis=None):
+    def __init__(self, basis=None, target=None):
         """Optimize1qGatesDecomposition initializer.
 
         Args:
             basis (list[str]): Basis gates to consider, e.g. `['u3', 'cx']`. For the effects
                 of this pass, the basis is the set intersection between the `basis` parameter
-                and the Euler basis.
+                and the Euler basis. Ignored if ``target`` is also specified.
+            target (Optional[Target]): The :class:`~.Target` object corresponding to the compilation
+                target. When specified, any argument specified for ``basis_gates`` is ignored.
         """
         super().__init__()
 
-        self._target_basis = basis
-        self._decomposers = None
+        self._basis_gates = basis
+        self._target = target
+        self._global_decomposers = None
+        self._local_decomposers_cache = {}
 
         if basis:
-            self._decomposers = {}
-            basis_set = set(basis)
-            euler_basis_gates = one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES
-            for euler_basis_name, gates in euler_basis_gates.items():
-                if set(gates).issubset(basis_set):
-                    basis_copy = copy.copy(self._decomposers)
-                    for base in basis_copy.keys():
-                        # check if gates are a superset of another basis
-                        if set(base).issubset(set(gates)):
-                            # if so, remove that basis
-                            del self._decomposers[base]
-                        # check if the gates are a subset of another basis
-                        elif set(gates).issubset(set(base)):
-                            # if so, don't bother
-                            break
-                    # if not a subset, add it to the list
-                    else:
-                        self._decomposers[
-                            tuple(gates)
-                        ] = one_qubit_decompose.OneQubitEulerDecomposer(euler_basis_name)
-
-    def _resynthesize_run(self, run):
+            self._global_decomposers = _possible_decomposers(set(basis))
+
+    def _resynthesize_run(self, run, qubit=None):
         """
         Resynthesizes one `run`, typically extracted via `dag.collect_1q_runs`.
 
-        Returns (basis, circuit) containing the newly synthesized circuit in the indicated basis, or
-        (None, None) if no synthesis routine applied.
+        Returns the newly synthesized circuit in the indicated basis, or None
+        if no synthesis routine applied.
         """
-
         operator = run[0].op.to_matrix()
         for gate in run[1:]:
             operator = gate.op.to_matrix().dot(operator)
 
-        new_circs = {k: v._decompose(operator) for k, v in self._decomposers.items()}
-
-        new_basis, new_circ = None, None
-        if len(new_circs) > 0:
-            new_basis, new_circ = min(new_circs.items(), key=lambda x: len(x[1]))
-
-        return new_basis, new_circ
-
-    def _substitution_checks(self, dag, old_run, new_circ, new_basis):
+        if self._target:
+            qubits_tuple = (qubit,)
+            if qubits_tuple in self._local_decomposers_cache:
+                decomposers = self._local_decomposers_cache[qubits_tuple]
+            else:
+                available_1q_basis = set(self._target.operation_names_for_qargs(qubits_tuple))
+                decomposers = _possible_decomposers(available_1q_basis)
+                self._local_decomposers_cache[qubits_tuple] = decomposers
+        else:
+            decomposers = self._global_decomposers
+
+        new_circs = [decomposer._decompose(operator) for decomposer in decomposers]
+
+        if len(new_circs) == 0:
+            return None
+        else:
+            return min(new_circs, key=partial(_error, target=self._target, qubit=qubit))
+
+    def _substitution_checks(self, dag, old_run, new_circ, basis, qubit):
         """
-        Returns `True` when it is recommended to replace `old_run` with `new_circ`.
+        Returns `True` when it is recommended to replace `old_run` with `new_circ` over `basis`.
         """
-
         if new_circ is None:
             return False
 
         # do we even have calibrations?
         has_cals_p = dag.calibrations is not None and len(dag.calibrations) > 0
-        # is this run in the target set of this particular decomposer and also uncalibrated?
-        rewriteable_and_in_basis_p = all(
-            g.name in new_basis and (not has_cals_p or not dag.has_calibration_for(g))
-            for g in old_run
-        )
         # does this run have uncalibrated gates?
         uncalibrated_p = not has_cals_p or any(not dag.has_calibration_for(g) for g in old_run)
         # does this run have gates not in the image of ._decomposers _and_ uncalibrated?
         uncalibrated_and_not_basis_p = any(
-            g.name not in self._target_basis and (not has_cals_p or not dag.has_calibration_for(g))
+            g.name not in basis and (not has_cals_p or not dag.has_calibration_for(g))
             for g in old_run
         )
 
-        if rewriteable_and_in_basis_p and len(old_run) < len(new_circ):
-            # NOTE: This is short-circuited on calibrated gates, which we're timid about
-            #       reducing.
-            logger.debug(
-                "Resynthesized \n\n"
-                + "\n".join([str(node.op) for node in old_run])
-                + "\n\nand got\n\n"
-                + "\n".join([str(node[0]) for node in new_circ])
-                + f"\n\nbut the original was native (for {self._target_basis}) and the new value "
-                "is longer.  This indicates an efficiency bug in synthesis.  Please report it by "
-                "opening an issue here: "
-                "https://github.com/Qiskit/qiskit-terra/issues/new/choose",
-                stacklevel=2,
-            )
-
         # if we're outside of the basis set, we're obligated to logically decompose.
         # if we're outside of the set of gates for which we have physical definitions,
         #    then we _try_ to decompose, using the results if we see improvement.
-        # NOTE: Here we use circuit length as a weak proxy for "improvement"; in reality,
-        #       we care about something more like fidelity at runtime, which would mean,
-        #       e.g., a preference for `RZGate`s over `RXGate`s.  In fact, users sometimes
-        #       express a preference for a "canonical form" of a circuit, which may come in
-        #       the form of some parameter values, also not visible at the level of circuit
-        #       length.  Since we don't have a framework for the caller to programmatically
-        #       express what they want here, we include some special casing for particular
-        #       gates which we've promised to normalize --- but this is fragile and should
-        #       ultimately be done away with.
         return (
             uncalibrated_and_not_basis_p
-            or (uncalibrated_p and len(old_run) > len(new_circ))
-            or isinstance(old_run[0].op, U3Gate)
+            or (
+                uncalibrated_p
+                and _error(new_circ, self._target, qubit) < _error(old_run, self._target, qubit)
+            )
+            or np.isclose(_error(new_circ, self._target, qubit), 0)
         )
 
     @control_flow.trivial_recurse
@@ -150,30 +124,63 @@ def run(self, dag):
         Returns:
             DAGCircuit: the optimized DAG.
         """
-        if self._decomposers is None:
-            logger.info("Skipping pass because no basis is set")
+        if self._basis_gates is None and self._target is None:
+            logger.info("Skipping pass because no basis or target is set")
             return dag
 
         runs = dag.collect_1q_runs()
+        qubit_indices = {bit: index for index, bit in enumerate(dag.qubits)}
         for run in runs:
-            # SPECIAL CASE: Don't bother to optimize single U3 gates which are in the basis set.
-            #     The U3 decomposer is only going to emit a sequence of length 1 anyhow.
-            if "u3" in self._target_basis and len(run) == 1 and isinstance(run[0].op, U3Gate):
-                # Toss U3 gates equivalent to the identity; there we get off easy.
-                if np.allclose(run[0].op.to_matrix(), np.eye(2), 1e-15, 0):
-                    dag.remove_op_node(run[0])
-                    continue
-                # We might rewrite into lower `u`s if they're available.
-                if "u2" not in self._target_basis and "u1" not in self._target_basis:
-                    continue
-
-            new_basis, new_circ = self._resynthesize_run(run)
-
-            if new_circ is not None and self._substitution_checks(dag, run, new_circ, new_basis):
+            qubit = qubit_indices[run[0].qargs[0]]
+            new_circ = self._resynthesize_run(run, qubit)
+
+            if self._target is None:
+                basis = self._basis_gates
+            else:
+                basis = self._target.operation_names_for_qargs((qubit,))
+
+            if new_circ is not None and self._substitution_checks(dag, run, new_circ, basis, qubit):
                 new_dag = circuit_to_dag(new_circ)
                 dag.substitute_node_with_dag(run[0], new_dag)
                 # Delete the other nodes in the run
                 for current_node in run[1:]:
                     dag.remove_op_node(current_node)
 
         return dag
+
+
+def _possible_decomposers(basis_set):
+    decomposers = []
+    euler_basis_gates = one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES
+    for euler_basis_name, gates in euler_basis_gates.items():
+        if set(gates).issubset(basis_set):
+            decomposer = one_qubit_decompose.OneQubitEulerDecomposer(euler_basis_name)
+            decomposers.append(decomposer)
+    return decomposers
+
+
+def _error(circuit, target, qubit):
+    """
+    Calculate a rough error for a `circuit` that runs on a specific
+    `qubit` of `target` (circuit could also be a list of DAGNodes)
+
+    Use basis errors from target if available, otherwise use length
+    of circuit as a weak proxy for error.
+    """
+    if target is None:
+        return len(circuit)
+    else:
+        if isinstance(circuit, list):
+            gate_fidelities = [
+                1 - getattr(target[node.name].get((qubit,)), "error", 0.0) for node in circuit
+            ]
+        else:
+            gate_fidelities = [
+                1 - getattr(target[inst.operation.name].get((qubit,)), "error", 0.0)
+                for inst in circuit
+            ]
+        gate_error = 1 - np.product(gate_fidelities)
+        if gate_error == 0.0:
+            return -100 + len(circuit)  # prefer shorter circuits among those with zero error
+        else:
+            return gate_error

qiskit/transpiler/preset_passmanagers/level1.py

@@ -178,7 +178,7 @@ def _vf2_match_not_found(property_set):
     def _opt_control(property_set):
         return (not property_set["depth_fixed_point"]) or (not property_set["size_fixed_point"])
 
-    _opt = [Optimize1qGatesDecomposition(basis_gates), CXCancellation()]
+    _opt = [Optimize1qGatesDecomposition(basis=basis_gates, target=target), CXCancellation()]
 
     unroll_3q = None
     # Build full pass manager

qiskit/transpiler/preset_passmanagers/level2.py

@@ -145,7 +145,7 @@ def _opt_control(property_set):
         return (not property_set["depth_fixed_point"]) or (not property_set["size_fixed_point"])
 
     _opt = [
-        Optimize1qGatesDecomposition(basis_gates),
+        Optimize1qGatesDecomposition(basis=basis_gates, target=target),
         CommutativeCancellation(basis_gates=basis_gates),
     ]
 

qiskit/transpiler/preset_passmanagers/level3.py

@@ -162,7 +162,7 @@ def _opt_control(property_set):
             plugin_config=unitary_synthesis_plugin_config,
             target=target,
         ),
-        Optimize1qGatesDecomposition(basis_gates),
+        Optimize1qGatesDecomposition(basis=basis_gates, target=target),
         CommutativeCancellation(),
     ]