Fix handling of globally defined operations in Target (#8925) (#8938)

This commit fixes the handling in the target for working with
instructions defined globally in the target. It adds handling to the
operation_names_for_qargs() and operations_for_qargs() methods to build
outputs that include globally defined operations instead of ignoring
them. Additionally the add_instruction() method is updated to validate
the input qargs to ensure that only valid qubits and qargs are allowed
to be added to the target for an instruction.

Fixes #8914

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 3a073c7)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

qiskit/transpiler/passes/layout/dense_layout.py

@@ -147,7 +147,7 @@ def _build_error_matrix(num_qubits, target=None, coupling_map=None, backend_prop
             error = 0.0
             ops = target.operation_names_for_qargs(qargs)
             for op in ops:
-                props = target[op][qargs]
+                props = target[op].get(qargs, None)
                 if props is not None and props.error is not None:
                     # Use max error rate to represent operation error
                     # on a qubit(s). If there is more than 1 operation available

qiskit/transpiler/target.py

@@ -181,6 +181,7 @@ class Target(Mapping):
         "_non_global_basis",
         "_non_global_strict_basis",
         "qubit_properties",
+        "_global_operations",
     )
 
     def __init__(
@@ -238,6 +239,8 @@ def __init__(
         self._gate_name_map = {}
         # A nested mapping of gate name -> qargs -> properties
         self._gate_map = {}
+        # A mapping of number of qubits to set of op names which are global
+        self._global_operations = defaultdict(set)
         # A mapping of qarg -> set(gate name)
         self._qarg_gate_map = defaultdict(set)
         self.dt = dt
@@ -350,8 +353,15 @@ def add_instruction(self, instruction, properties=None, name=None):
         if is_class:
             qargs_val = {None: None}
         else:
+            if None in properties:
+                self._global_operations[instruction.num_qubits].add(instruction_name)
             qargs_val = {}
             for qarg in properties:
+                if qarg is not None and len(qarg) != instruction.num_qubits:
+                    raise TranspilerError(
+                        f"The number of qubits for {instruction} does not match the number "
+                        f"of qubits in the properties dictionary: {qarg}"
+                    )
                 if qarg is not None:
                     self.num_qubits = max(self.num_qubits, max(qarg) + 1)
                 qargs_val[qarg] = properties[qarg]
@@ -530,7 +540,8 @@ def operations_for_qargs(self, qargs):
         Args:
             qargs (tuple): A qargs tuple of the qubits to get the gates that apply
                 to it. For example, ``(0,)`` will return the set of all
-                instructions that apply to qubit 0.
+                instructions that apply to qubit 0. If set to ``None`` this will
+                return any globally defined operations in the target.
         Returns:
             list: The list of :class:`~qiskit.circuit.Instruction` instances
             that apply to the specified qarg. This may also be a class if
@@ -539,30 +550,44 @@ def operations_for_qargs(self, qargs):
         Raises:
             KeyError: If qargs is not in target
         """
-        if qargs not in self._qarg_gate_map:
+        if qargs is not None and any(x not in range(0, self.num_qubits) for x in qargs):
             raise KeyError(f"{qargs} not in target.")
         res = [self._gate_name_map[x] for x in self._qarg_gate_map[qargs]]
-        if None in self._qarg_gate_map:
-            res += [self._gate_name_map[x] for x in self._qarg_gate_map[None]]
-        return res
+        if qargs is not None:
+            res += self._global_operations.get(len(qargs), list())
+        for op in self._gate_name_map.values():
+            if inspect.isclass(op):
+                res.append(op)
+        if not res:
+            raise KeyError(f"{qargs} not in target.")
+        return list(res)
 
     def operation_names_for_qargs(self, qargs):
         """Get the operation names for a specified qargs tuple
 
         Args:
             qargs (tuple): A qargs tuple of the qubits to get the gates that apply
                 to it. For example, ``(0,)`` will return the set of all
-                instructions that apply to qubit 0.
+                instructions that apply to qubit 0. If set to ``None`` this will
+                return the names for any globally defined operations in the target.
         Returns:
             set: The set of operation names that apply to the specified
             `qargs``.
 
         Raises:
             KeyError: If qargs is not in target
         """
-        if qargs not in self._qarg_gate_map:
+        if qargs is not None and any(x not in range(0, self.num_qubits) for x in qargs):
+            raise KeyError(f"{qargs} not in target.")
+        res = self._qarg_gate_map.get(qargs, set())
+        if qargs is not None:
+            res.update(self._global_operations.get(len(qargs), set()))
+        for name, op in self._gate_name_map.items():
+            if inspect.isclass(op):
+                res.add(name)
+        if not res:
             raise KeyError(f"{qargs} not in target.")
-        return self._qarg_gate_map[qargs]
+        return res
 
     def instruction_supported(
         self, operation_name=None, qargs=None, operation_class=None, parameters=None

releasenotes/notes/fix-global-inst-qarg-method-target-a9188e172ea7f325.yaml

@@ -0,0 +1,31 @@
+---
+fixes:
+  - |
+    Fixed handling of globally defined instructions for the :class:`~.Target`
+    class. Previously, two methods, :meth:`~.Target.operations_for_qargs` and
+    :meth:`~.Target.operation_names_for_qargs` would ignore/incorrectly handle
+    any globally defined ideal operations present in the target. For example::
+
+        from qiskit.transpiler import Target
+        from qiskit.circuit.library import CXGate
+
+        target = Target(num_qubits=5)
+        target.add_instruction(CXGate())
+        names = target.operation_names_for_qargs((1, 2))
+        ops = target.operations_for_qargs((1, 2))
+
+    will now return ``{"cx"}`` for ``names`` and ``[CXGate()]`` for ``ops``
+    instead of raising a ``KeyError`` or an empty return.
+  - |
+    Fixed an issue in the :meth:`.Target.add_instruction` method where it
+    would previously have accepted an argument with an invalid number of
+    qubits as part of the ``properties`` argument. For example::
+
+        from qiskit.transpiler import Target
+        from qiskit.circuit.library import CXGate
+
+        target = Target()
+        target.add_instruction(CXGate(), {(0, 1, 2): None})
+
+    This will now correctly raise a ``TranspilerError`` instead of causing
+    runtime issues when interacting with the target.

test/python/transpiler/test_target.py

@@ -38,6 +38,7 @@
 from qiskit.transpiler.coupling import CouplingMap
 from qiskit.transpiler.instruction_durations import InstructionDurations
 from qiskit.transpiler.timing_constraints import TimingConstraints
+from qiskit.transpiler.exceptions import TranspilerError
 from qiskit.transpiler import Target
 from qiskit.transpiler import InstructionProperties
 from qiskit.test import QiskitTestCase
@@ -398,6 +399,30 @@ def test_get_instructions_for_qargs(self):
         for gate in ideal_sim_expected:
             self.assertIn(gate, self.ideal_sim_target.operations_for_qargs(None))
 
+    def test_get_operation_for_qargs_global(self):
+        expected = [
+            RXGate(self.theta),
+            RYGate(self.theta),
+            RZGate(self.theta),
+            RGate(self.theta, self.phi),
+            Measure(),
+        ]
+        res = self.aqt_target.operations_for_qargs((0,))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+        expected = [RXXGate(self.theta)]
+        res = self.aqt_target.operations_for_qargs((0, 1))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+
+    def test_get_invalid_operations_for_qargs(self):
+        with self.assertRaises(KeyError):
+            self.ibm_target.operations_for_qargs((0, 102))
+        with self.assertRaises(KeyError):
+            self.ibm_target.operations_for_qargs(None)
+
     def test_get_operation_names_for_qargs(self):
         with self.assertRaises(KeyError):
             self.empty_target.operation_names_for_qargs((0,))
@@ -416,6 +441,20 @@ def test_get_operation_names_for_qargs(self):
         for gate in ideal_sim_expected:
             self.assertIn(gate, self.ideal_sim_target.operation_names_for_qargs(None))
 
+    def test_get_operation_names_for_qargs_invalid_qargs(self):
+        with self.assertRaises(KeyError):
+            self.ibm_target.operation_names_for_qargs((0, 102))
+        with self.assertRaises(KeyError):
+            self.ibm_target.operation_names_for_qargs(None)
+
+    def test_get_operation_names_for_qargs_global_insts(self):
+        expected = {"r", "rx", "rz", "ry", "measure"}
+        self.assertEqual(self.aqt_target.operation_names_for_qargs((0,)), expected)
+        expected = {
+            "rxx",
+        }
+        self.assertEqual(self.aqt_target.operation_names_for_qargs((0, 1)), expected)
+
     def test_coupling_map(self):
         self.assertEqual(
             CouplingMap().get_edges(), self.empty_target.build_coupling_map().get_edges()
@@ -1374,6 +1413,84 @@ def test_instruction_names(self):
             {"u", "cx", "measure", "if_else", "while_loop", "for_loop"},
         )
 
+    def test_operations_for_qargs(self):
+        expected = [
+            IGate(),
+            RZGate(self.theta),
+            SXGate(),
+            XGate(),
+            Measure(),
+            IfElseOp,
+            ForLoopOp,
+            WhileLoopOp,
+        ]
+        res = self.ibm_target.operations_for_qargs((0,))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+        expected = [
+            CXGate(),
+            IfElseOp,
+            ForLoopOp,
+            WhileLoopOp,
+        ]
+        res = self.ibm_target.operations_for_qargs((0, 1))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+        expected = [
+            RXGate(self.theta),
+            RYGate(self.theta),
+            RZGate(self.theta),
+            RGate(self.theta, self.phi),
+            Measure(),
+            IfElseOp,
+            ForLoopOp,
+            WhileLoopOp,
+        ]
+        res = self.aqt_target.operations_for_qargs((0,))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+        expected = [RXXGate(self.theta), IfElseOp, ForLoopOp, WhileLoopOp]
+        res = self.aqt_target.operations_for_qargs((0, 1))
+        self.assertEqual(len(expected), len(res))
+        for x in expected:
+            self.assertIn(x, res)
+
+    def test_operation_names_for_qargs(self):
+        expected = {
+            "id",
+            "rz",
+            "sx",
+            "x",
+            "measure",
+            "if_else",
+            "for_loop",
+            "while_loop",
+        }
+        self.assertEqual(expected, self.ibm_target.operation_names_for_qargs((0,)))
+        expected = {
+            "cx",
+            "if_else",
+            "for_loop",
+            "while_loop",
+        }
+        self.assertEqual(expected, self.ibm_target.operation_names_for_qargs((0, 1)))
+        expected = {
+            "rx",
+            "ry",
+            "rz",
+            "r",
+            "measure",
+            "if_else",
+            "for_loop",
+            "while_loop",
+        }
+        self.assertEqual(self.aqt_target.operation_names_for_qargs((0,)), expected)
+        expected = {"rxx", "if_else", "for_loop", "while_loop"}
+        self.assertEqual(self.aqt_target.operation_names_for_qargs((0, 1)), expected)
+
     def test_operations(self):
         ibm_expected = [
             RZGate(self.theta),
@@ -1411,6 +1528,12 @@ def test_operations(self):
         for gate in fake_expected:
             self.assertIn(gate, self.target_global_gates_only.operations)
 
+    def test_add_invalid_instruction(self):
+        inst_props = {(0, 1, 2, 3): None}
+        target = Target()
+        with self.assertRaises(TranspilerError):
+            target.add_instruction(CXGate(), inst_props)
+
     def test_instructions(self):
         ibm_expected = [
             (IGate(), (0,)),