From 6c46cc31924488487ccaabebd8cdfcb3637cba04 Mon Sep 17 00:00:00 2001 From: Naoki Kanazawa Date: Fri, 17 Feb 2023 23:32:37 +0900 Subject: [PATCH] Update Target to use CalibrationEntry to create inst map (#9597) * Bug fix for InstructionScheduleMap.has_custom_gate. InstructionProperties._calibration now only takes CalibrationEntry and use this instance to create an inst map. * Fix repr not to call .calibration * Review comment Co-authored-by: Matthew Treinish * Remove getattr --------- Co-authored-by: Matthew Treinish Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit ecb6f9a536ff516e93905792f518d9b7e9731fd2) --- qiskit/pulse/calibration_entries.py | 9 ++++- qiskit/pulse/instruction_schedule_map.py | 8 ++--- qiskit/transpiler/target.py | 27 ++++++++++----- ...-instmap-from-target-f38962c3fd03e5d3.yaml | 7 ++++ .../pulse/test_instruction_schedule_map.py | 8 +++-- test/python/transpiler/test_target.py | 33 ++++++++++++++++++- 6 files changed, 75 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-instmap-from-target-f38962c3fd03e5d3.yaml diff --git a/qiskit/pulse/calibration_entries.py b/qiskit/pulse/calibration_entries.py index 3aff515834ca..d0f8376580ba 100644 --- a/qiskit/pulse/calibration_entries.py +++ b/qiskit/pulse/calibration_entries.py @@ -123,6 +123,9 @@ def _parse_argument(self): def define(self, definition: Union[Schedule, ScheduleBlock]): self._definition = definition + # add metadata + if "publisher" not in definition.metadata: + definition.metadata["publisher"] = CalibrationPublisher.QISKIT self._parse_argument() def get_signature(self) -> inspect.Signature: @@ -185,7 +188,11 @@ def get_schedule(self, *args, **kwargs) -> Union[Schedule, ScheduleBlock]: except TypeError as ex: raise PulseError("Assigned parameter doesn't match with function signature.") from ex - return self._definition(**to_bind.arguments) + schedule = self._definition(**to_bind.arguments) + # add metadata + if "publisher" not in schedule.metadata: + schedule.metadata["publisher"] = CalibrationPublisher.QISKIT + return schedule def __eq__(self, other): # We cannot evaluate function equality without parsing python AST. diff --git a/qiskit/pulse/instruction_schedule_map.py b/qiskit/pulse/instruction_schedule_map.py index dec2b0aec9f4..c5d144d11ecd 100644 --- a/qiskit/pulse/instruction_schedule_map.py +++ b/qiskit/pulse/instruction_schedule_map.py @@ -10,6 +10,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. +# pylint: disable=unused-import + """ A convenient way to track reusable subschedules by name and qubit. @@ -34,11 +36,12 @@ from qiskit.circuit.instruction import Instruction from qiskit.circuit.parameterexpression import ParameterExpression from qiskit.pulse.calibration_entries import ( - CalibrationPublisher, CalibrationEntry, ScheduleDef, CallableDef, PulseQobjDef, + # for backward compatibility + CalibrationPublisher, ) from qiskit.pulse.exceptions import PulseError from qiskit.pulse.schedule import Schedule, ScheduleBlock @@ -248,9 +251,6 @@ def add( # generate signature if isinstance(schedule, (Schedule, ScheduleBlock)): entry = ScheduleDef(arguments) - # add metadata - if "publisher" not in schedule.metadata: - schedule.metadata["publisher"] = CalibrationPublisher.QISKIT elif callable(schedule): if arguments: warnings.warn( diff --git a/qiskit/transpiler/target.py b/qiskit/transpiler/target.py index a3872fa26b0d..3325fda4c0c8 100644 --- a/qiskit/transpiler/target.py +++ b/qiskit/transpiler/target.py @@ -29,7 +29,7 @@ from qiskit.circuit.parameter import Parameter from qiskit.pulse.instruction_schedule_map import InstructionScheduleMap -from qiskit.pulse.calibration_entries import CalibrationEntry +from qiskit.pulse.calibration_entries import CalibrationEntry, ScheduleDef from qiskit.pulse.schedule import Schedule, ScheduleBlock from qiskit.transpiler.coupling import CouplingMap from qiskit.transpiler.exceptions import TranspilerError @@ -71,20 +71,25 @@ def __init__( set of qubits. calibration: The pulse representation of the instruction. """ + self._calibration = None + self.duration = duration self.error = error - self._calibration = calibration + self.calibration = calibration @property def calibration(self): """The pulse representation of the instruction.""" - if isinstance(self._calibration, CalibrationEntry): - return self._calibration.get_schedule() - return self._calibration + return self._calibration.get_schedule() @calibration.setter def calibration(self, calibration: Union[Schedule, ScheduleBlock, CalibrationEntry]): - self._calibration = calibration + if isinstance(calibration, (Schedule, ScheduleBlock)): + new_entry = ScheduleDef() + new_entry.define(calibration) + else: + new_entry = calibration + self._calibration = new_entry def __repr__(self): return ( @@ -530,8 +535,12 @@ def instruction_schedule_map(self): out_inst_schedule_map = InstructionScheduleMap() for instruction, qargs in self._gate_map.items(): for qarg, properties in qargs.items(): - if properties is not None and properties.calibration is not None: - out_inst_schedule_map.add(instruction, qarg, properties.calibration) + # Directly getting CalibrationEntry not to invoke .get_schedule(). + # This keeps PulseQobjDef un-parsed. + cal_entry = getattr(properties, "_calibration", None) + if cal_entry is not None: + # Use fast-path to add entries to the inst map. + out_inst_schedule_map._add(instruction, qarg, cal_entry) self._instruction_schedule_map = out_inst_schedule_map return out_inst_schedule_map @@ -1008,7 +1017,7 @@ def __str__(self): error = getattr(props, "error", None) if error is not None: prop_str_pieces.append(f"\t\t\tError Rate: {error}\n") - schedule = getattr(props, "calibration", None) + schedule = getattr(props, "_calibration", None) if schedule is not None: prop_str_pieces.append("\t\t\tWith pulse schedule calibration\n") extra_props = getattr(props, "properties", None) diff --git a/releasenotes/notes/fix-instmap-from-target-f38962c3fd03e5d3.yaml b/releasenotes/notes/fix-instmap-from-target-f38962c3fd03e5d3.yaml new file mode 100644 index 000000000000..a94e629fc306 --- /dev/null +++ b/releasenotes/notes/fix-instmap-from-target-f38962c3fd03e5d3.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed an issue with the :meth:`.InstructionScheduleMap.has_custom_gate` method, + where it would always return ``True`` when the :class:`~.InstructionScheduleMap` + object was created by :class:`.Target`. + Fixed `#9595 `__ diff --git a/test/python/pulse/test_instruction_schedule_map.py b/test/python/pulse/test_instruction_schedule_map.py index 1179b805f3dd..414a1d6566a5 100644 --- a/test/python/pulse/test_instruction_schedule_map.py +++ b/test/python/pulse/test_instruction_schedule_map.py @@ -30,7 +30,7 @@ ShiftPhase, Constant, ) -from qiskit.pulse.instruction_schedule_map import CalibrationPublisher +from qiskit.pulse.calibration_entries import CalibrationPublisher from qiskit.pulse.channels import DriveChannel from qiskit.qobj import PulseQobjInstruction from qiskit.qobj.converters import QobjToInstructionConverter @@ -602,8 +602,12 @@ def test_has_custom_gate(self): self.assertFalse(instmap.has_custom_gate()) - # add something + # add custom schedule some_sched = Schedule() instmap.add("u3", (0,), some_sched) self.assertTrue(instmap.has_custom_gate()) + + # delete custom schedule + instmap.remove("u3", (0,)) + self.assertFalse(instmap.has_custom_gate()) diff --git a/test/python/transpiler/test_target.py b/test/python/transpiler/test_target.py index efb3e4a2ab94..e67fd06c8caf 100644 --- a/test/python/transpiler/test_target.py +++ b/test/python/transpiler/test_target.py @@ -35,6 +35,7 @@ from qiskit.circuit.parameter import Parameter from qiskit import pulse from qiskit.pulse.instruction_schedule_map import InstructionScheduleMap +from qiskit.pulse.calibration_entries import CalibrationPublisher from qiskit.transpiler.coupling import CouplingMap from qiskit.transpiler.instruction_durations import InstructionDurations from qiskit.transpiler.timing_constraints import TimingConstraints @@ -42,7 +43,7 @@ from qiskit.transpiler import Target from qiskit.transpiler import InstructionProperties from qiskit.test import QiskitTestCase -from qiskit.providers.fake_provider import FakeBackendV2, FakeMumbaiFractionalCX +from qiskit.providers.fake_provider import FakeBackendV2, FakeMumbaiFractionalCX, FakeGeneva class TestTarget(QiskitTestCase): @@ -1228,6 +1229,36 @@ def test_timing_constraints(self): f"{getattr(generated_constraints, i)}!={getattr(expected_constraints, i)}", ) + def test_default_instmap_has_no_custom_gate(self): + backend = FakeGeneva() + target = backend.target + + # This copies .calibraiton of InstructionProperties of each instruction + # This must not convert PulseQobj to Schedule during this. + # See qiskit-terra/#9595 + inst_map = target.instruction_schedule_map() + self.assertFalse(inst_map.has_custom_gate()) + + # Get pulse schedule. This generates Schedule provided by backend. + sched = inst_map.get("sx", (0,)) + self.assertEqual(sched.metadata["publisher"], CalibrationPublisher.BACKEND_PROVIDER) + self.assertFalse(inst_map.has_custom_gate()) + + # Update target with custom instruction. This is user provided schedule. + new_prop = InstructionProperties( + duration=self.custom_sx_q0.duration, + error=None, + calibration=self.custom_sx_q0, + ) + target.update_instruction_properties(instruction="sx", qargs=(0,), properties=new_prop) + inst_map = target.instruction_schedule_map() + self.assertTrue(inst_map.has_custom_gate()) + + empty = InstructionProperties() + target.update_instruction_properties(instruction="sx", qargs=(0,), properties=empty) + inst_map = target.instruction_schedule_map() + self.assertFalse(inst_map.has_custom_gate()) + class TestGlobalVariableWidthOperations(QiskitTestCase): def setUp(self):