Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pulse gate transpiler pass to use target. #9587

Merged
merged 16 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions qiskit/compiler/transpiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,8 @@ def _parse_transpile_args(
durations = _parse_instruction_durations(backend, instruction_durations, dt, circuits)
timing_constraints = _parse_timing_constraints(backend, timing_constraints, num_circuits)
target = _parse_target(backend, target)
if inst_map is not None and inst_map.has_custom_gate() and target is not None:
target.update_from_instruction_schedule_map(inst_map)
if scheduling_method and any(d is None for d in durations):
raise TranspilerError(
"Transpiling a circuit with a scheduling method"
Expand Down
2 changes: 1 addition & 1 deletion qiskit/providers/models/pulsedefaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def __init__(

for inst in cmd_def:
entry = PulseQobjDef(converter=self.converter, name=inst.name)
entry.define(inst.sequence)
entry.define(inst.sequence, user_provided=False)
self.instruction_schedule_map._add(
instruction_name=inst.name,
qubits=tuple(inst.qubits),
Expand Down
107 changes: 75 additions & 32 deletions qiskit/pulse/calibration_entries.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ class CalibrationEntry(metaclass=ABCMeta):
"""A metaclass of a calibration entry."""

@abstractmethod
def define(self, definition: Any):
def define(self, definition: Any, user_provided: bool):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a breaking API change for any downstream usage of CalibrationEntry right? Like if you have an existing CalibrationEntry subclass in 0.23.x then when upgrading to 0.24.0 that class's define method signature will no long match the abstract interface definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, all subclasses provide default value for user_provided to conform to previous default behavior. Currently in 0.23.x, this flag is implicitly determined by type information and only PulseQobjDef is recognized as a backend provided entry. However one may want to provide backend calibration as ScheduleDef, to bypass cumbersome JSON data generation. This could be a separate PR though.

"""Attach definition to the calibration entry.

Args:
definition: Definition of this entry.
user_provided: If this entry is defined by user.
"""
pass

Expand All @@ -64,6 +65,12 @@ def get_schedule(self, *args, **kwargs) -> Union[Schedule, ScheduleBlock]:
"""
pass

@property
@abstractmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do this without breaking compatibility? Wouldn't adding a new required abstract method break any downstream usage of CalibrationEntry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 33b76b3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this has been fixed (or if it was the fix got undone at some point), because this is still a new abstractmethod which will cause a hard failure for any existing implementations of this interface.

Copy link
Contributor Author

@nkanazawa1989 nkanazawa1989 Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously I implemented an abstractmethod to return schedule duration. Note that schedule duration is computed here for added schedule from the inst map, but this requires Qobj parsing that is the biggest performance bottleneck for non-pulse users. So I have implemented a custom (complicated) method that computes the duration without parsing Qobj.
https://github.com/Qiskit/qiskit-terra/blob/bf3bb96cebc141f79ce46b5938724eef30778c8e/qiskit/transpiler/target.py#L449-L457

New abstract method just returns whether it is provided by backend or by end-user. If your schedule is provided from backend, you should know duration of it and we don't need to update duration in the first place. This avoids unnecessary Qobj parsing for duration. If it is user-provided one, usually they don't define schedule with JSON format, and no parsing overhead.

def user_provided(self) -> bool:
"""Return if this entry is user defined."""
pass


class ScheduleDef(CalibrationEntry):
"""In-memory Qiskit Pulse representation.
Expand All @@ -90,6 +97,11 @@ def __init__(self, arguments: Optional[Sequence[str]] = None):

self._definition = None
self._signature = None
self._user_provided = None

@property
def user_provided(self) -> bool:
return self._user_provided

def _parse_argument(self):
"""Generate signature from program and user provided argument names."""
Expand Down Expand Up @@ -120,35 +132,48 @@ def _parse_argument(self):
)
self._signature = signature

def define(self, definition: Union[Schedule, ScheduleBlock]):
def define(
self,
definition: Union[Schedule, ScheduleBlock],
user_provided: bool = True,
):
self._definition = definition
# add metadata
if "publisher" not in definition.metadata:
definition.metadata["publisher"] = CalibrationPublisher.QISKIT
self._parse_argument()
self._user_provided = user_provided

def get_signature(self) -> inspect.Signature:
return self._signature

def get_schedule(self, *args, **kwargs) -> Union[Schedule, ScheduleBlock]:
if not args and not kwargs:
return self._definition
try:
to_bind = self.get_signature().bind_partial(*args, **kwargs)
except TypeError as ex:
raise PulseError("Assigned parameter doesn't match with schedule parameters.") from ex
value_dict = {}
for param in self._definition.parameters:
# Schedule allows partial bind. This results in parameterized Schedule.
out = self._definition
else:
try:
value_dict[param] = to_bind.arguments[param.name]
except KeyError:
pass
return self._definition.assign_parameters(value_dict, inplace=False)
to_bind = self.get_signature().bind_partial(*args, **kwargs)
except TypeError as ex:
raise PulseError(
"Assigned parameter doesn't match with schedule parameters."
) from ex
value_dict = {}
for param in self._definition.parameters:
# Schedule allows partial bind. This results in parameterized Schedule.
try:
value_dict[param] = to_bind.arguments[param.name]
except KeyError:
pass
out = self._definition.assign_parameters(value_dict, inplace=False)
if "publisher" not in out.metadata:
if self.user_provided:
out.metadata["publisher"] = CalibrationPublisher.QISKIT
else:
out.metadata["publisher"] = CalibrationPublisher.BACKEND_PROVIDER
return out

def __eq__(self, other):
# This delegates equality check to Schedule or ScheduleBlock.
return self._definition == other._definition
if hasattr(other, "_definition"):
return self._definition == other._definition
return False

def __str__(self):
out = f"Schedule {self._definition.name}"
Expand All @@ -171,10 +196,20 @@ def __init__(self):
"""Define an empty entry."""
self._definition = None
self._signature = None
self._user_provided = None

@property
def user_provided(self) -> bool:
return self._user_provided

def define(self, definition: Callable):
def define(
self,
definition: Callable,
user_provided: bool = True,
):
self._definition = definition
self._signature = inspect.signature(definition)
self._user_provided = user_provided

def get_signature(self) -> inspect.Signature:
return self._signature
Expand All @@ -186,17 +221,20 @@ def get_schedule(self, *args, **kwargs) -> Union[Schedule, ScheduleBlock]:
to_bind.apply_defaults()
except TypeError as ex:
raise PulseError("Assigned parameter doesn't match with function signature.") from ex

schedule = self._definition(**to_bind.arguments)
# add metadata
if "publisher" not in schedule.metadata:
schedule.metadata["publisher"] = CalibrationPublisher.QISKIT
return schedule
out = self._definition(**to_bind.arguments)
if "publisher" not in out.metadata:
if self.user_provided:
out.metadata["publisher"] = CalibrationPublisher.QISKIT
else:
out.metadata["publisher"] = CalibrationPublisher.BACKEND_PROVIDER
return out

def __eq__(self, other):
# We cannot evaluate function equality without parsing python AST.
# This simply compares wether they are the same object.
return self._definition is other._definition
# This simply compares weather they are the same object.
if hasattr(other, "_definition"):
return self._definition == other._definition
return False

def __str__(self):
params_str = ", ".join(self.get_signature().parameters.keys())
Expand Down Expand Up @@ -237,14 +275,17 @@ def _build_schedule(self):
for qobj_inst in self._source:
for qiskit_inst in self._converter._get_sequences(qobj_inst):
schedule.insert(qobj_inst.t0, qiskit_inst, inplace=True)
schedule.metadata["publisher"] = CalibrationPublisher.BACKEND_PROVIDER

self._definition = schedule
self._parse_argument()

def define(self, definition: List[PulseQobjInstruction]):
def define(
self,
definition: List[PulseQobjInstruction],
user_provided: bool = False,
):
# This doesn't generate signature immediately, because of lazy schedule build.
self._source = definition
self._user_provided = user_provided

def get_signature(self) -> inspect.Signature:
if self._definition is None:
Expand All @@ -261,9 +302,11 @@ def __eq__(self, other):
# If both objects are Qobj just check Qobj equality.
return self._source == other._source
if isinstance(other, ScheduleDef) and self._definition is None:
# To compare with other scheudle def, this also generates schedule object from qobj.
# To compare with other schedule def, this also generates schedule object from qobj.
self._build_schedule()
return self._definition == other._definition
if hasattr(other, "_definition"):
return self._definition == other._definition
return False

def __str__(self):
if self._definition is None:
Expand Down
6 changes: 3 additions & 3 deletions qiskit/pulse/instruction_schedule_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
CalibrationEntry,
ScheduleDef,
CallableDef,
PulseQobjDef,
# for backward compatibility
PulseQobjDef,
CalibrationPublisher,
)
from qiskit.pulse.exceptions import PulseError
Expand Down Expand Up @@ -77,7 +77,7 @@ def has_custom_gate(self) -> bool:
"""Return ``True`` if the map has user provided instruction."""
for qubit_inst in self._map.values():
for entry in qubit_inst.values():
if not isinstance(entry, PulseQobjDef):
if entry.user_provided:
return True
return False

Expand Down Expand Up @@ -264,7 +264,7 @@ def add(
"Supplied schedule must be one of the Schedule, ScheduleBlock or a "
"callable that outputs a schedule."
)
entry.define(schedule)
entry.define(schedule, user_provided=True)
self._add(instruction, qubits, entry)

def _add(
Expand Down
28 changes: 17 additions & 11 deletions qiskit/transpiler/passes/calibration/pulse_gate.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
from typing import List, Union

from qiskit.circuit import Instruction as CircuitInst
from qiskit.pulse import (
Schedule,
ScheduleBlock,
)
from qiskit.pulse import Schedule, ScheduleBlock
from qiskit.pulse.instruction_schedule_map import InstructionScheduleMap
from qiskit.transpiler.target import Target
from qiskit.transpiler.exceptions import TranspilerError

from .base_builder import CalibrationBuilder

Expand Down Expand Up @@ -59,13 +57,18 @@ def __init__(
Args:
inst_map: Instruction schedule map that user may override.
target: The :class:`~.Target` representing the target backend, if both
``inst_map`` and this are specified then this argument will take
precedence and ``inst_map`` will be ignored.
``inst_map`` and this are specified then it updates instructions
in the ``target`` with ``inst_map``.
"""
super().__init__()
self.inst_map = inst_map
if target:
self.inst_map = target.instruction_schedule_map()

if inst_map is None and target is None:
raise TranspilerError("inst_map and target cannot be None simulataneously.")

if target is None:
target = Target()
target.update_from_instruction_schedule_map(inst_map)
self.target = target

def supported(self, node_op: CircuitInst, qubits: List) -> bool:
"""Determine if a given node supports the calibration.
Expand All @@ -77,7 +80,7 @@ def supported(self, node_op: CircuitInst, qubits: List) -> bool:
Returns:
Return ``True`` is calibration can be provided.
"""
return self.inst_map.has(instruction=node_op.name, qubits=qubits)
return self.target.calibration_supported(node_op.name, tuple(qubits))

def get_calibration(self, node_op: CircuitInst, qubits: List) -> Union[Schedule, ScheduleBlock]:
"""Gets the calibrated schedule for the given instruction and qubits.
Expand All @@ -88,5 +91,8 @@ def get_calibration(self, node_op: CircuitInst, qubits: List) -> Union[Schedule,

Returns:
Return Schedule of target gate instruction.

Raises:
TranspilerError: When node is parameterized and calibration is raw schedule object.
"""
return self.inst_map.get(node_op.name, qubits, *node_op.params)
return self.target.get_calibration(node_op.name, tuple(qubits), *node_op.params)
Loading