From 450108c0e5421109548bd4618b5c9b8c4c2ac796 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 18 Oct 2022 08:15:45 +0000 Subject: [PATCH] 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 3a073c7c5e6d0ab870cb659dd4bcc4836888c1ad) Co-authored-by: Matthew Treinish --- .../transpiler/passes/layout/dense_layout.py | 2 +- qiskit/transpiler/target.py | 41 ++++-- ...t-qarg-method-target-a9188e172ea7f325.yaml | 31 +++++ test/python/transpiler/test_target.py | 123 ++++++++++++++++++ 4 files changed, 188 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/fix-global-inst-qarg-method-target-a9188e172ea7f325.yaml diff --git a/qiskit/transpiler/passes/layout/dense_layout.py b/qiskit/transpiler/passes/layout/dense_layout.py index 62edf53f6267..6b75b6d09a7b 100644 --- a/qiskit/transpiler/passes/layout/dense_layout.py +++ b/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 diff --git a/qiskit/transpiler/target.py b/qiskit/transpiler/target.py index c48731208d54..769ad9fe56a4 100644 --- a/qiskit/transpiler/target.py +++ b/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,12 +550,17 @@ 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 @@ -552,7 +568,8 @@ def operation_names_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 the names for any globally defined operations in the target. Returns: set: The set of operation names that apply to the specified `qargs``. @@ -560,9 +577,17 @@ def operation_names_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._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 diff --git a/releasenotes/notes/fix-global-inst-qarg-method-target-a9188e172ea7f325.yaml b/releasenotes/notes/fix-global-inst-qarg-method-target-a9188e172ea7f325.yaml new file mode 100644 index 000000000000..acdb18c1bdf3 --- /dev/null +++ b/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. diff --git a/test/python/transpiler/test_target.py b/test/python/transpiler/test_target.py index dd1ca7ba0a0c..ccf879605420 100644 --- a/test/python/transpiler/test_target.py +++ b/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,)),