From f05d4b4ad9951dc61284a20e032b02156d9cf98e Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Thu, 30 Jun 2022 22:27:08 +0100 Subject: [PATCH 01/13] Fixes #7078: Add is_schedulable property to qiskit.pulse.Channel and don't apply a delay to it if True RegisterSlot and MemorySlot have this set to False. The default implementation in Channel sets it to True. --- qiskit/pulse/channels.py | 15 +++++++++++++++ qiskit/pulse/instructions/delay.py | 6 ++++++ qiskit/pulse/transforms/canonicalization.py | 2 ++ 3 files changed, 23 insertions(+) diff --git a/qiskit/pulse/channels.py b/qiskit/pulse/channels.py index 175d93ef8e87..94adce9c53bc 100644 --- a/qiskit/pulse/channels.py +++ b/qiskit/pulse/channels.py @@ -140,6 +140,11 @@ def name(self) -> str: """Return the shorthand alias for this channel, which is based on its type and index.""" return f"{self.__class__.prefix}{self._index}" + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return True + def __repr__(self): return f"{self.__class__.__name__}({self._index})" @@ -207,6 +212,11 @@ class MemorySlot(Channel): prefix = "m" + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return False + class RegisterSlot(Channel): """Classical resister slot channels represent classical registers (low-latency classical @@ -214,3 +224,8 @@ class RegisterSlot(Channel): """ prefix = "c" + + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return False diff --git a/qiskit/pulse/instructions/delay.py b/qiskit/pulse/instructions/delay.py index f045a4880ff3..9f663796bbc6 100644 --- a/qiskit/pulse/instructions/delay.py +++ b/qiskit/pulse/instructions/delay.py @@ -15,6 +15,7 @@ from qiskit.circuit import ParameterExpression from qiskit.pulse.channels import Channel +from qiskit.pulse.exceptions import PulseError from qiskit.pulse.instructions.instruction import Instruction @@ -48,8 +49,13 @@ def __init__( duration: Length of time of the delay in terms of dt. channel: The channel that will have the delay. name: Name of the delay for display purposes. + + Raises: + PulseError: If `channel` cannot be delayed because it cannot be scheduled. """ super().__init__(operands=(duration, channel), name=name) + if not channel.is_schedulable: + raise PulseError("Cannot delay chanel because it is not schedulable.") @property def channel(self) -> Channel: diff --git a/qiskit/pulse/transforms/canonicalization.py b/qiskit/pulse/transforms/canonicalization.py index a019de2eda27..12d8214962d5 100644 --- a/qiskit/pulse/transforms/canonicalization.py +++ b/qiskit/pulse/transforms/canonicalization.py @@ -475,6 +475,8 @@ def pad( channels = channels or schedule.channels for channel in channels: + if not channel.is_schedulable: + continue if channel not in schedule.channels: schedule |= instructions.Delay(until, channel) continue From 0b1a11136e869b0f6476be5187d8978332a8fb40 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Fri, 8 Jul 2022 23:17:23 +0100 Subject: [PATCH 02/13] Fixes #7078: Introduce ClassicalIOChannel as a subclass of pulse.Channel RegisterSlot, MemorySlot and SnapshotChannel now derive from this class. There are some tests to fix. --- qiskit/pulse/channels.py | 27 +++++++-------------- qiskit/pulse/instructions/delay.py | 12 ++++++--- qiskit/pulse/instructions/directives.py | 12 +++++++++ qiskit/pulse/transforms/canonicalization.py | 3 ++- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/qiskit/pulse/channels.py b/qiskit/pulse/channels.py index 94adce9c53bc..c88fddadd77b 100644 --- a/qiskit/pulse/channels.py +++ b/qiskit/pulse/channels.py @@ -140,11 +140,6 @@ def name(self) -> str: """Return the shorthand alias for this channel, which is based on its type and index.""" return f"{self.__class__.prefix}{self._index}" - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return True - def __repr__(self): return f"{self.__class__.__name__}({self._index})" @@ -170,6 +165,12 @@ class PulseChannel(Channel, metaclass=ABCMeta): pass +class ClassicalIOChannel(Channel, metaclass=ABCMeta): + """Base class of classical IO channels. These cannot have instructions scheduled on them.""" + + pass + + class DriveChannel(PulseChannel): """Drive channels transmit signals to qubits which enact gate operations.""" @@ -197,7 +198,7 @@ class AcquireChannel(Channel): prefix = "a" -class SnapshotChannel(Channel): +class SnapshotChannel(ClassicalIOChannel): """Snapshot channels are used to specify instructions for simulators.""" prefix = "s" @@ -207,25 +208,15 @@ def __init__(self): super().__init__(0) -class MemorySlot(Channel): +class MemorySlot(ClassicalIOChannel): """Memory slot channels represent classical memory storage.""" prefix = "m" - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return False - -class RegisterSlot(Channel): +class RegisterSlot(ClassicalIOChannel): """Classical resister slot channels represent classical registers (low-latency classical memory). """ prefix = "c" - - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return False diff --git a/qiskit/pulse/instructions/delay.py b/qiskit/pulse/instructions/delay.py index 9f663796bbc6..c68893a02d03 100644 --- a/qiskit/pulse/instructions/delay.py +++ b/qiskit/pulse/instructions/delay.py @@ -14,7 +14,7 @@ from typing import Optional, Union, Tuple from qiskit.circuit import ParameterExpression -from qiskit.pulse.channels import Channel +from qiskit.pulse.channels import Channel, ClassicalIOChannel from qiskit.pulse.exceptions import PulseError from qiskit.pulse.instructions.instruction import Instruction @@ -51,11 +51,15 @@ def __init__( name: Name of the delay for display purposes. Raises: - PulseError: If `channel` cannot be delayed because it cannot be scheduled. + PulseError: If `channel` cannot be delayed because it is a classical IO channel. """ super().__init__(operands=(duration, channel), name=name) - if not channel.is_schedulable: - raise PulseError("Cannot delay chanel because it is not schedulable.") + if isinstance(channel, ClassicalIOChannel): + raise PulseError( + "Cannot delay channel {0} because it is a classical IO channel.".format( + channel.name + ) + ) @property def channel(self) -> Channel: diff --git a/qiskit/pulse/instructions/directives.py b/qiskit/pulse/instructions/directives.py index d09b69823332..9460af014394 100644 --- a/qiskit/pulse/instructions/directives.py +++ b/qiskit/pulse/instructions/directives.py @@ -16,6 +16,8 @@ from typing import Optional, Tuple from qiskit.pulse import channels as chans +from qiskit.pulse.channels import ClassicalIOChannel +from qiskit.pulse.exceptions import PulseError from qiskit.pulse.instructions import instruction @@ -44,7 +46,17 @@ def __init__(self, *channels: chans.Channel, name: Optional[str] = None): Args: channels: The channel that the barrier applies to. name: Name of the directive for display purposes. + Raises: + PulseError: If one of the `channels` cannot be blocked because it is a classical IO channel. """ + for channel in channels: + if isinstance(channel, ClassicalIOChannel): + raise PulseError( + "Cannot block channel {0} because it is a classical IO channel.".format( + channel.name + ) + ) + super().__init__(operands=tuple(channels), name=name) @property diff --git a/qiskit/pulse/transforms/canonicalization.py b/qiskit/pulse/transforms/canonicalization.py index 12d8214962d5..4e2409ffa354 100644 --- a/qiskit/pulse/transforms/canonicalization.py +++ b/qiskit/pulse/transforms/canonicalization.py @@ -18,6 +18,7 @@ import numpy as np from qiskit.pulse import channels as chans, exceptions, instructions +from qiskit.pulse.channels import ClassicalIOChannel from qiskit.pulse.exceptions import PulseError from qiskit.pulse.exceptions import UnassignedDurationError from qiskit.pulse.instruction_schedule_map import InstructionScheduleMap @@ -475,7 +476,7 @@ def pad( channels = channels or schedule.channels for channel in channels: - if not channel.is_schedulable: + if isinstance(channel, ClassicalIOChannel): continue if channel not in schedule.channels: schedule |= instructions.Delay(until, channel) From 34ffb0b150722163b30bc59d3a1a1a10599febc9 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Fri, 17 Jun 2022 16:13:51 +0100 Subject: [PATCH 03/13] Fixes #7078: Make Acquire.channels only return the channel --- qiskit/pulse/instructions/acquire.py | 4 ++-- qiskit/scheduler/lowering.py | 8 +++++--- test/python/pulse/test_schedule.py | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/qiskit/pulse/instructions/acquire.py b/qiskit/pulse/instructions/acquire.py index 9555e9317938..0322d7742e21 100644 --- a/qiskit/pulse/instructions/acquire.py +++ b/qiskit/pulse/instructions/acquire.py @@ -88,9 +88,9 @@ def channel(self) -> AcquireChannel: return self.operands[1] @property - def channels(self) -> Tuple[Union[AcquireChannel, MemorySlot, RegisterSlot]]: + def channels(self) -> Tuple[AcquireChannel]: """Returns the channels that this schedule uses.""" - return tuple(self.operands[ind] for ind in (1, 2, 3) if self.operands[ind] is not None) + return self.channel, @property def duration(self) -> Union[int, ParameterExpression]: diff --git a/qiskit/scheduler/lowering.py b/qiskit/scheduler/lowering.py index 4eb4b14b1078..5e30e23718d5 100644 --- a/qiskit/scheduler/lowering.py +++ b/qiskit/scheduler/lowering.py @@ -22,9 +22,9 @@ from qiskit.circuit.measure import Measure from qiskit.circuit.quantumcircuit import QuantumCircuit from qiskit.exceptions import QiskitError -from qiskit.pulse import Schedule +from qiskit.pulse import Schedule, Acquire from qiskit.pulse import instructions as pulse_inst -from qiskit.pulse.channels import AcquireChannel, MemorySlot, DriveChannel +from qiskit.pulse.channels import AcquireChannel, DriveChannel from qiskit.pulse.exceptions import PulseError from qiskit.pulse.macros import measure from qiskit.scheduler.config import ScheduleConfig @@ -79,7 +79,9 @@ def get_measure_schedule(qubit_mem_slots: Dict[int, int]) -> CircuitPulseDef: meas_q = target_qobj_transform(meas_q) acquire_q = meas_q.filter(channels=[AcquireChannel(qubit)]) mem_slot_index = [ - chan.index for chan in acquire_q.channels if isinstance(chan, MemorySlot) + acquire[1].mem_slot.index + for acquire in acquire_q.instructions + if isinstance(acquire[1], Acquire) and not (acquire[1].mem_slot is None) ][0] if mem_slot_index != qubit_mem_slots[qubit]: raise KeyError( diff --git a/test/python/pulse/test_schedule.py b/test/python/pulse/test_schedule.py index e8f6a2f5a84f..296f73fac63f 100644 --- a/test/python/pulse/test_schedule.py +++ b/test/python/pulse/test_schedule.py @@ -332,7 +332,7 @@ def test_schedule_with_acquire_on_single_qubit(self): ) self.assertEqual(len(sched_single.instructions), 2) - self.assertEqual(len(sched_single.channels), 6) + self.assertEqual(len(sched_single.channels), 2) def test_parametric_commands_in_sched(self): """Test that schedules can be built with parametric commands.""" From 40aba7109fded370cdd6f1af1d4a39727cc433a0 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Sun, 19 Jun 2022 14:06:22 +0100 Subject: [PATCH 04/13] Fixes #7078: Execute black --- qiskit/pulse/instructions/acquire.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/pulse/instructions/acquire.py b/qiskit/pulse/instructions/acquire.py index 0322d7742e21..a7abdd33ccf9 100644 --- a/qiskit/pulse/instructions/acquire.py +++ b/qiskit/pulse/instructions/acquire.py @@ -90,7 +90,7 @@ def channel(self) -> AcquireChannel: @property def channels(self) -> Tuple[AcquireChannel]: """Returns the channels that this schedule uses.""" - return self.channel, + return (self.channel,) @property def duration(self) -> Union[int, ParameterExpression]: From 3395b5e4c88b46a76b32d81772836b38d2a3b75d Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Mon, 20 Jun 2022 22:45:28 +0100 Subject: [PATCH 05/13] Fixes #7078: Put slots in new tuple in acquire --- qiskit/pulse/instructions/acquire.py | 5 +++++ qiskit/pulse/instructions/instruction.py | 5 +++++ qiskit/pulse/schedule.py | 10 +++++++--- test/python/pulse/test_schedule.py | 2 +- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/qiskit/pulse/instructions/acquire.py b/qiskit/pulse/instructions/acquire.py index a7abdd33ccf9..4e91723f00da 100644 --- a/qiskit/pulse/instructions/acquire.py +++ b/qiskit/pulse/instructions/acquire.py @@ -92,6 +92,11 @@ def channels(self) -> Tuple[AcquireChannel]: """Returns the channels that this schedule uses.""" return (self.channel,) + @property + def slots(self) -> Tuple[Union[MemorySlot, RegisterSlot]]: + """Returns the slots that this schedule uses.""" + return tuple(self.operands[ind] for ind in (2, 3) if self.operands[ind] is not None) + @property def duration(self) -> Union[int, ParameterExpression]: """Duration of this instruction.""" diff --git a/qiskit/pulse/instructions/instruction.py b/qiskit/pulse/instructions/instruction.py index 5ddffab05e82..0585f6dcfa4b 100644 --- a/qiskit/pulse/instructions/instruction.py +++ b/qiskit/pulse/instructions/instruction.py @@ -86,6 +86,11 @@ def start_time(self) -> int: """Relative begin time of this instruction.""" return 0 + @property + def slots(self) -> Tuple[Channel]: + """Returns any slots that this schedule uses""" + return () + @property def stop_time(self) -> int: """Relative end time of this instruction.""" diff --git a/qiskit/pulse/schedule.py b/qiskit/pulse/schedule.py index 843e6030ebf3..687463d9cf90 100644 --- a/qiskit/pulse/schedule.py +++ b/qiskit/pulse/schedule.py @@ -523,7 +523,9 @@ def _add_timeslots(self, time: int, schedule: "ScheduleComponent") -> None: other_timeslots = _get_timeslots(schedule) self._duration = max(self._duration, time + schedule.duration) - for channel in schedule.channels: + for channel in schedule.channels + ( + schedule.slots if isinstance(schedule, Instruction) else () + ): if channel not in self._timeslots: if time == 0: self._timeslots[channel] = copy.copy(other_timeslots[channel]) @@ -575,7 +577,9 @@ def _remove_timeslots(self, time: int, schedule: "ScheduleComponent"): if not isinstance(time, int): raise PulseError("Schedule start time must be an integer.") - for channel in schedule.channels: + for channel in schedule.channels + ( + schedule.slots if isinstance(schedule, Instruction) else () + ): if channel not in self._timeslots: raise PulseError(f"The channel {channel} is not present in the schedule") @@ -1495,7 +1499,7 @@ def _get_timeslots(schedule: "ScheduleComponent") -> TimeSlots: if isinstance(schedule, Instruction): duration = schedule.duration instruction_duration_validation(duration) - timeslots = {channel: [(0, duration)] for channel in schedule.channels} + timeslots = {channel: [(0, duration)] for channel in schedule.channels + schedule.slots} elif isinstance(schedule, Schedule): timeslots = schedule.timeslots else: diff --git a/test/python/pulse/test_schedule.py b/test/python/pulse/test_schedule.py index 296f73fac63f..e8f6a2f5a84f 100644 --- a/test/python/pulse/test_schedule.py +++ b/test/python/pulse/test_schedule.py @@ -332,7 +332,7 @@ def test_schedule_with_acquire_on_single_qubit(self): ) self.assertEqual(len(sched_single.instructions), 2) - self.assertEqual(len(sched_single.channels), 2) + self.assertEqual(len(sched_single.channels), 6) def test_parametric_commands_in_sched(self): """Test that schedules can be built with parametric commands.""" From 9bf6e09d4d107be4cfeacf63a1f0ce5f19f0e7bc Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Thu, 30 Jun 2022 21:53:26 +0100 Subject: [PATCH 06/13] Fixes #7078: Add is_schedulable property to qiskit.pulse.Channel and don't apply a delay to it if True RegisterSlot and MemorySlot have this set to False. The default implementation in Channel sets it to True. --- qiskit/pulse/channels.py | 15 +++++++++++++++ qiskit/pulse/instructions/acquire.py | 9 ++------- qiskit/pulse/instructions/instruction.py | 5 ----- qiskit/pulse/schedule.py | 10 +++------- qiskit/scheduler/lowering.py | 8 +++----- 5 files changed, 23 insertions(+), 24 deletions(-) diff --git a/qiskit/pulse/channels.py b/qiskit/pulse/channels.py index c88fddadd77b..8b11d760b0d2 100644 --- a/qiskit/pulse/channels.py +++ b/qiskit/pulse/channels.py @@ -140,6 +140,11 @@ def name(self) -> str: """Return the shorthand alias for this channel, which is based on its type and index.""" return f"{self.__class__.prefix}{self._index}" + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return True + def __repr__(self): return f"{self.__class__.__name__}({self._index})" @@ -213,6 +218,11 @@ class MemorySlot(ClassicalIOChannel): prefix = "m" + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return False + class RegisterSlot(ClassicalIOChannel): """Classical resister slot channels represent classical registers (low-latency classical @@ -220,3 +230,8 @@ class RegisterSlot(ClassicalIOChannel): """ prefix = "c" + + @property + def is_schedulable(self) -> bool: + """Return whether this channel can be independently scheduled""" + return False diff --git a/qiskit/pulse/instructions/acquire.py b/qiskit/pulse/instructions/acquire.py index 4e91723f00da..9555e9317938 100644 --- a/qiskit/pulse/instructions/acquire.py +++ b/qiskit/pulse/instructions/acquire.py @@ -88,14 +88,9 @@ def channel(self) -> AcquireChannel: return self.operands[1] @property - def channels(self) -> Tuple[AcquireChannel]: + def channels(self) -> Tuple[Union[AcquireChannel, MemorySlot, RegisterSlot]]: """Returns the channels that this schedule uses.""" - return (self.channel,) - - @property - def slots(self) -> Tuple[Union[MemorySlot, RegisterSlot]]: - """Returns the slots that this schedule uses.""" - return tuple(self.operands[ind] for ind in (2, 3) if self.operands[ind] is not None) + return tuple(self.operands[ind] for ind in (1, 2, 3) if self.operands[ind] is not None) @property def duration(self) -> Union[int, ParameterExpression]: diff --git a/qiskit/pulse/instructions/instruction.py b/qiskit/pulse/instructions/instruction.py index 0585f6dcfa4b..5ddffab05e82 100644 --- a/qiskit/pulse/instructions/instruction.py +++ b/qiskit/pulse/instructions/instruction.py @@ -86,11 +86,6 @@ def start_time(self) -> int: """Relative begin time of this instruction.""" return 0 - @property - def slots(self) -> Tuple[Channel]: - """Returns any slots that this schedule uses""" - return () - @property def stop_time(self) -> int: """Relative end time of this instruction.""" diff --git a/qiskit/pulse/schedule.py b/qiskit/pulse/schedule.py index 687463d9cf90..843e6030ebf3 100644 --- a/qiskit/pulse/schedule.py +++ b/qiskit/pulse/schedule.py @@ -523,9 +523,7 @@ def _add_timeslots(self, time: int, schedule: "ScheduleComponent") -> None: other_timeslots = _get_timeslots(schedule) self._duration = max(self._duration, time + schedule.duration) - for channel in schedule.channels + ( - schedule.slots if isinstance(schedule, Instruction) else () - ): + for channel in schedule.channels: if channel not in self._timeslots: if time == 0: self._timeslots[channel] = copy.copy(other_timeslots[channel]) @@ -577,9 +575,7 @@ def _remove_timeslots(self, time: int, schedule: "ScheduleComponent"): if not isinstance(time, int): raise PulseError("Schedule start time must be an integer.") - for channel in schedule.channels + ( - schedule.slots if isinstance(schedule, Instruction) else () - ): + for channel in schedule.channels: if channel not in self._timeslots: raise PulseError(f"The channel {channel} is not present in the schedule") @@ -1499,7 +1495,7 @@ def _get_timeslots(schedule: "ScheduleComponent") -> TimeSlots: if isinstance(schedule, Instruction): duration = schedule.duration instruction_duration_validation(duration) - timeslots = {channel: [(0, duration)] for channel in schedule.channels + schedule.slots} + timeslots = {channel: [(0, duration)] for channel in schedule.channels} elif isinstance(schedule, Schedule): timeslots = schedule.timeslots else: diff --git a/qiskit/scheduler/lowering.py b/qiskit/scheduler/lowering.py index 5e30e23718d5..4eb4b14b1078 100644 --- a/qiskit/scheduler/lowering.py +++ b/qiskit/scheduler/lowering.py @@ -22,9 +22,9 @@ from qiskit.circuit.measure import Measure from qiskit.circuit.quantumcircuit import QuantumCircuit from qiskit.exceptions import QiskitError -from qiskit.pulse import Schedule, Acquire +from qiskit.pulse import Schedule from qiskit.pulse import instructions as pulse_inst -from qiskit.pulse.channels import AcquireChannel, DriveChannel +from qiskit.pulse.channels import AcquireChannel, MemorySlot, DriveChannel from qiskit.pulse.exceptions import PulseError from qiskit.pulse.macros import measure from qiskit.scheduler.config import ScheduleConfig @@ -79,9 +79,7 @@ def get_measure_schedule(qubit_mem_slots: Dict[int, int]) -> CircuitPulseDef: meas_q = target_qobj_transform(meas_q) acquire_q = meas_q.filter(channels=[AcquireChannel(qubit)]) mem_slot_index = [ - acquire[1].mem_slot.index - for acquire in acquire_q.instructions - if isinstance(acquire[1], Acquire) and not (acquire[1].mem_slot is None) + chan.index for chan in acquire_q.channels if isinstance(chan, MemorySlot) ][0] if mem_slot_index != qubit_mem_slots[qubit]: raise KeyError( From a21787976553e108e44587b6eafdd9d3e66ce85c Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Sat, 9 Jul 2022 12:20:47 +0100 Subject: [PATCH 07/13] Fixes #7078: Remove is_schedulable property from qiskit.pulse.channel --- qiskit/pulse/channels.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/qiskit/pulse/channels.py b/qiskit/pulse/channels.py index 8b11d760b0d2..c88fddadd77b 100644 --- a/qiskit/pulse/channels.py +++ b/qiskit/pulse/channels.py @@ -140,11 +140,6 @@ def name(self) -> str: """Return the shorthand alias for this channel, which is based on its type and index.""" return f"{self.__class__.prefix}{self._index}" - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return True - def __repr__(self): return f"{self.__class__.__name__}({self._index})" @@ -218,11 +213,6 @@ class MemorySlot(ClassicalIOChannel): prefix = "m" - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return False - class RegisterSlot(ClassicalIOChannel): """Classical resister slot channels represent classical registers (low-latency classical @@ -230,8 +220,3 @@ class RegisterSlot(ClassicalIOChannel): """ prefix = "c" - - @property - def is_schedulable(self) -> bool: - """Return whether this channel can be independently scheduled""" - return False From cfd81c7232d04fc9c8aab441e392c2a31bb5b35d Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Sun, 17 Jul 2022 14:09:21 +0100 Subject: [PATCH 08/13] Fixes #7078: Put checks in SetPhase, ShiftPhase and SetPhase. Remove check from Delay. --- qiskit/pulse/instructions/delay.py | 9 +-------- qiskit/pulse/instructions/directives.py | 12 ------------ qiskit/pulse/instructions/frequency.py | 8 +++++++- qiskit/pulse/instructions/phase.py | 15 ++++++++++++++- qiskit/pulse/instructions/play.py | 2 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/qiskit/pulse/instructions/delay.py b/qiskit/pulse/instructions/delay.py index c68893a02d03..edaa406f4fcb 100644 --- a/qiskit/pulse/instructions/delay.py +++ b/qiskit/pulse/instructions/delay.py @@ -14,8 +14,7 @@ from typing import Optional, Union, Tuple from qiskit.circuit import ParameterExpression -from qiskit.pulse.channels import Channel, ClassicalIOChannel -from qiskit.pulse.exceptions import PulseError +from qiskit.pulse.channels import Channel from qiskit.pulse.instructions.instruction import Instruction @@ -54,12 +53,6 @@ def __init__( PulseError: If `channel` cannot be delayed because it is a classical IO channel. """ super().__init__(operands=(duration, channel), name=name) - if isinstance(channel, ClassicalIOChannel): - raise PulseError( - "Cannot delay channel {0} because it is a classical IO channel.".format( - channel.name - ) - ) @property def channel(self) -> Channel: diff --git a/qiskit/pulse/instructions/directives.py b/qiskit/pulse/instructions/directives.py index 9460af014394..d09b69823332 100644 --- a/qiskit/pulse/instructions/directives.py +++ b/qiskit/pulse/instructions/directives.py @@ -16,8 +16,6 @@ from typing import Optional, Tuple from qiskit.pulse import channels as chans -from qiskit.pulse.channels import ClassicalIOChannel -from qiskit.pulse.exceptions import PulseError from qiskit.pulse.instructions import instruction @@ -46,17 +44,7 @@ def __init__(self, *channels: chans.Channel, name: Optional[str] = None): Args: channels: The channel that the barrier applies to. name: Name of the directive for display purposes. - Raises: - PulseError: If one of the `channels` cannot be blocked because it is a classical IO channel. """ - for channel in channels: - if isinstance(channel, ClassicalIOChannel): - raise PulseError( - "Cannot block channel {0} because it is a classical IO channel.".format( - channel.name - ) - ) - super().__init__(operands=tuple(channels), name=name) @property diff --git a/qiskit/pulse/instructions/frequency.py b/qiskit/pulse/instructions/frequency.py index 09233080b593..7e2f5b5f7d65 100644 --- a/qiskit/pulse/instructions/frequency.py +++ b/qiskit/pulse/instructions/frequency.py @@ -16,7 +16,7 @@ from typing import Optional, Union, Tuple from qiskit.circuit.parameterexpression import ParameterExpression -from qiskit.pulse.channels import PulseChannel +from qiskit.pulse.channels import PulseChannel, PulseError from qiskit.pulse.instructions.instruction import Instruction @@ -46,7 +46,13 @@ def __init__( frequency: New frequency of the channel in Hz. channel: The channel this instruction operates on. name: Name of this set channel frequency instruction. + Raises: + PulseError: If channel is not a PulseChannel. """ + if not isinstance(channel, PulseChannel): + raise PulseError( + "The `channel` argument to `SetFrequency` must be of type `channels.PulseChannel`." + ) if not isinstance(frequency, ParameterExpression): frequency = float(frequency) super().__init__(operands=(frequency, channel), name=name) diff --git a/qiskit/pulse/instructions/phase.py b/qiskit/pulse/instructions/phase.py index b9067486d923..d8db037a5aa1 100644 --- a/qiskit/pulse/instructions/phase.py +++ b/qiskit/pulse/instructions/phase.py @@ -18,7 +18,7 @@ from typing import Optional, Union, Tuple from qiskit.circuit import ParameterExpression -from qiskit.pulse.channels import PulseChannel +from qiskit.pulse.channels import PulseChannel, PulseError from qiskit.pulse.instructions.instruction import Instruction @@ -52,7 +52,14 @@ def __init__( phase: The rotation angle in radians. channel: The channel this instruction operates on. name: Display name for this instruction. + + Raises: + PulseError: If channel is not a PulseChannel. """ + if not isinstance(channel, PulseChannel): + raise PulseError( + "The `channel` argument to `ShiftPhase` must be of type `channels.PulseChannel`." + ) super().__init__(operands=(phase, channel), name=name) @property @@ -108,7 +115,13 @@ def __init__( phase: The rotation angle in radians. channel: The channel this instruction operates on. name: Display name for this instruction. + Raises: + PulseError: If channel is not a PulseChannel. """ + if not isinstance(channel, PulseChannel): + raise PulseError( + "The `channel` argument to `SetPhase` must be of type `channels.PulseChannel`." + ) super().__init__(operands=(phase, channel), name=name) @property diff --git a/qiskit/pulse/instructions/play.py b/qiskit/pulse/instructions/play.py index 2a8536643e76..41effc205d83 100644 --- a/qiskit/pulse/instructions/play.py +++ b/qiskit/pulse/instructions/play.py @@ -41,7 +41,7 @@ def __init__(self, pulse: Pulse, channel: PulseChannel, name: Optional[str] = No name: Name of the instruction for display purposes. Defaults to ``pulse.name``. Raises: - PulseError: If pulse is not a Pulse type. + PulseError: If pulse is not a Pulse type, or channel is not a PulseChannel. """ if not isinstance(pulse, Pulse): raise PulseError("The `pulse` argument to `Play` must be of type `library.Pulse`.") From c66a6e35814fcbc61bb1d6a9d476f0f7c1868529 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Sun, 17 Jul 2022 15:58:29 +0100 Subject: [PATCH 09/13] Fixes #7078: Add unit tests for restrictions on Classical IO channels --- qiskit/pulse/instructions/frequency.py | 6 +++ test/python/pulse/test_channels.py | 6 ++- test/python/pulse/test_instructions.py | 63 ++++++++++++++++++++++++++ test/python/pulse/test_transforms.py | 19 +++++++- 4 files changed, 92 insertions(+), 2 deletions(-) diff --git a/qiskit/pulse/instructions/frequency.py b/qiskit/pulse/instructions/frequency.py index 7e2f5b5f7d65..dfff24ce9b48 100644 --- a/qiskit/pulse/instructions/frequency.py +++ b/qiskit/pulse/instructions/frequency.py @@ -99,9 +99,15 @@ def __init__( frequency: Frequency shift of the channel in Hz. channel: The channel this instruction operates on. name: Name of this set channel frequency instruction. + Raises: + PulseError: If channel is not a PulseChannel. """ if not isinstance(frequency, ParameterExpression): frequency = float(frequency) + if not isinstance(channel, PulseChannel): + raise PulseError( + "The `channel` argument to `SetFrequency` must be of type `channels.PulseChannel`." + ) super().__init__(operands=(frequency, channel), name=name) @property diff --git a/test/python/pulse/test_channels.py b/test/python/pulse/test_channels.py index 3637a1cea121..b4a24f6573c6 100644 --- a/test/python/pulse/test_channels.py +++ b/test/python/pulse/test_channels.py @@ -17,8 +17,9 @@ from qiskit.pulse.channels import ( AcquireChannel, Channel, - DriveChannel, + ClassicalIOChannel, ControlChannel, + DriveChannel, MeasureChannel, MemorySlot, PulseChannel, @@ -76,6 +77,7 @@ def test_default(self): self.assertEqual(memory_slot.index, 123) self.assertEqual(memory_slot.name, "m123") + self.assertTrue(isinstance(memory_slot, ClassicalIOChannel)) class TestRegisterSlot(QiskitTestCase): @@ -87,6 +89,7 @@ def test_default(self): self.assertEqual(register_slot.index, 123) self.assertEqual(register_slot.name, "c123") + self.assertTrue(isinstance(register_slot, ClassicalIOChannel)) class TestSnapshotChannel(QiskitTestCase): @@ -98,6 +101,7 @@ def test_default(self): self.assertEqual(snapshot_channel.index, 0) self.assertEqual(snapshot_channel.name, "s0") + self.assertTrue(isinstance(snapshot_channel, ClassicalIOChannel)) class TestDriveChannel(QiskitTestCase): diff --git a/test/python/pulse/test_instructions.py b/test/python/pulse/test_instructions.py index a5284c27ed2f..111ebd35f4ae 100644 --- a/test/python/pulse/test_instructions.py +++ b/test/python/pulse/test_instructions.py @@ -156,6 +156,64 @@ def test_freq(self): ) self.assertEqual(repr(set_freq), "SetFrequency(4500000000.0, DriveChannel(1), name='test')") + def test_freq_non_pulse_channel(self): + """Test set frequency constructor with illegal channel""" + with self.assertRaises(exceptions.PulseError): + instructions.SetFrequency(4.5e9, channels.RegisterSlot(1), name="test") + + +class TestShiftFrequency(QiskitTestCase): + """Shift frequency tests.""" + + def test_shift_freq(self): + """Test shift frequency basic functionality.""" + shift_freq = instructions.ShiftFrequency(4.5e9, channels.DriveChannel(1), name="test") + + self.assertIsInstance(shift_freq.id, int) + self.assertEqual(shift_freq.duration, 0) + self.assertEqual(shift_freq.frequency, 4.5e9) + self.assertEqual(shift_freq.operands, (4.5e9, channels.DriveChannel(1))) + self.assertEqual( + shift_freq, instructions.ShiftFrequency(4.5e9, channels.DriveChannel(1), name="test") + ) + self.assertNotEqual( + shift_freq, instructions.ShiftFrequency(4.5e8, channels.DriveChannel(1), name="test") + ) + self.assertEqual( + repr(shift_freq), "ShiftFrequency(4500000000.0, DriveChannel(1), name='test')" + ) + + def test_freq_non_pulse_channel(self): + """Test shift frequency constructor with illegal channel""" + with self.assertRaises(exceptions.PulseError): + instructions.ShiftFrequency(4.5e9, channels.RegisterSlot(1), name="test") + + +class TestSetPhase(QiskitTestCase): + """Test the instruction construction.""" + + def test_default(self): + """Test basic SetPhase.""" + set_phase = instructions.SetPhase(1.57, channels.DriveChannel(0)) + + self.assertIsInstance(set_phase.id, int) + self.assertEqual(set_phase.name, None) + self.assertEqual(set_phase.duration, 0) + self.assertEqual(set_phase.phase, 1.57) + self.assertEqual(set_phase.operands, (1.57, channels.DriveChannel(0))) + self.assertEqual( + set_phase, instructions.SetPhase(1.57, channels.DriveChannel(0), name="test") + ) + self.assertNotEqual( + set_phase, instructions.SetPhase(1.57j, channels.DriveChannel(0), name="test") + ) + self.assertEqual(repr(set_phase), "SetPhase(1.57, DriveChannel(0))") + + def test_set_phase_non_pulse_channel(self): + """Test shift phase constructor with illegal channel""" + with self.assertRaises(exceptions.PulseError): + instructions.SetPhase(1.57, channels.RegisterSlot(1), name="test") + class TestShiftPhase(QiskitTestCase): """Test the instruction construction.""" @@ -177,6 +235,11 @@ def test_default(self): ) self.assertEqual(repr(shift_phase), "ShiftPhase(1.57, DriveChannel(0))") + def test_shift_phase_non_pulse_channel(self): + """Test shift phase constructor with illegal channel""" + with self.assertRaises(exceptions.PulseError): + instructions.ShiftPhase(1.57, channels.RegisterSlot(1), name="test") + class TestSnapshot(QiskitTestCase): """Snapshot tests.""" diff --git a/test/python/pulse/test_transforms.py b/test/python/pulse/test_transforms.py index 49038afdb096..1cfb28f55245 100644 --- a/test/python/pulse/test_transforms.py +++ b/test/python/pulse/test_transforms.py @@ -29,7 +29,13 @@ Constant, ) from qiskit.pulse import transforms, instructions -from qiskit.pulse.channels import MemorySlot, DriveChannel, AcquireChannel +from qiskit.pulse.channels import ( + MemorySlot, + DriveChannel, + AcquireChannel, + RegisterSlot, + SnapshotChannel, +) from qiskit.pulse.instructions import directives from qiskit.test import QiskitTestCase from qiskit.providers.fake_provider import FakeOpenPulse2Q @@ -349,6 +355,17 @@ def test_padding_prepended_delay(self): self.assertEqual(transforms.pad(sched, until=30, inplace=True), ref_sched) + def test_pad_no_delay_on_classical_io_channels(self): + """Test padding does not apply to classical IO channels.""" + delay = 10 + sched = ( + Delay(delay, MemorySlot(0)).shift(20) + + Delay(delay, RegisterSlot(0)).shift(10) + + Delay(delay, SnapshotChannel()) + ) + + self.assertEqual(transforms.pad(sched, until=15), sched) + def get_pulse_ids(schedules: List[Schedule]) -> Set[int]: """Returns ids of pulses used in Schedules.""" From 87f0b2725d0eed3ba58eff19417a7bc4a353d413 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Sun, 17 Jul 2022 16:39:56 +0100 Subject: [PATCH 10/13] Fixes #7078: Add unit tests for abstract nature of ClassicalIOChannel --- test/python/pulse/test_channels.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/python/pulse/test_channels.py b/test/python/pulse/test_channels.py index b4a24f6573c6..604202fb5754 100644 --- a/test/python/pulse/test_channels.py +++ b/test/python/pulse/test_channels.py @@ -68,8 +68,17 @@ def test_channel_hash(self): self.assertEqual(hash_1, hash_2) +class TestClassicalIOChannel(QiskitTestCase): + """Test base classical IO channel.""" + + def test_cannot_be_instantiated(self): + """Test base classical IO channel cannot be instantiated.""" + with self.assertRaises(NotImplementedError): + ClassicalIOChannel(0) + + class TestMemorySlot(QiskitTestCase): - """AcquireChannel tests.""" + """MemorySlot tests.""" def test_default(self): """Test default memory slot.""" From e3f7634ce9703e3da925db7c8d12a4c42160b6d0 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Mon, 18 Jul 2022 09:04:36 +0100 Subject: [PATCH 11/13] Fixes #7078: Tidying after self-review. --- qiskit/pulse/instructions/delay.py | 5 +---- qiskit/pulse/instructions/frequency.py | 2 +- test/python/pulse/test_transforms.py | 8 +++++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/qiskit/pulse/instructions/delay.py b/qiskit/pulse/instructions/delay.py index edaa406f4fcb..1b911c350731 100644 --- a/qiskit/pulse/instructions/delay.py +++ b/qiskit/pulse/instructions/delay.py @@ -47,10 +47,7 @@ def __init__( Args: duration: Length of time of the delay in terms of dt. channel: The channel that will have the delay. - name: Name of the delay for display purposes. - - Raises: - PulseError: If `channel` cannot be delayed because it is a classical IO channel. + name: Name of the delay for display purposes.k """ super().__init__(operands=(duration, channel), name=name) diff --git a/qiskit/pulse/instructions/frequency.py b/qiskit/pulse/instructions/frequency.py index dfff24ce9b48..15866e0ae79a 100644 --- a/qiskit/pulse/instructions/frequency.py +++ b/qiskit/pulse/instructions/frequency.py @@ -106,7 +106,7 @@ def __init__( frequency = float(frequency) if not isinstance(channel, PulseChannel): raise PulseError( - "The `channel` argument to `SetFrequency` must be of type `channels.PulseChannel`." + "The `channel` argument to `ShiftFrequency` must be of type `channels.PulseChannel`." ) super().__init__(operands=(frequency, channel), name=name) diff --git a/test/python/pulse/test_transforms.py b/test/python/pulse/test_transforms.py index 1cfb28f55245..e19d024c2b4f 100644 --- a/test/python/pulse/test_transforms.py +++ b/test/python/pulse/test_transforms.py @@ -364,7 +364,13 @@ def test_pad_no_delay_on_classical_io_channels(self): + Delay(delay, SnapshotChannel()) ) - self.assertEqual(transforms.pad(sched, until=15), sched) + ref_sched = ( + Delay(delay, MemorySlot(0)).shift(20) + + Delay(delay, RegisterSlot(0)).shift(10) + + Delay(delay, SnapshotChannel()) + ) + + self.assertEqual(transforms.pad(sched, until=15), ref_sched) def get_pulse_ids(schedules: List[Schedule]) -> Set[int]: From 9208063e6471da3387241ebac1269280dd79d8c7 Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Mon, 18 Jul 2022 23:28:39 +0100 Subject: [PATCH 12/13] Fixes #7078: Add release note. --- ...oduce-classical-io-channel-0a616e6ca75b7687.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 releasenotes/notes/introduce-classical-io-channel-0a616e6ca75b7687.yaml diff --git a/releasenotes/notes/introduce-classical-io-channel-0a616e6ca75b7687.yaml b/releasenotes/notes/introduce-classical-io-channel-0a616e6ca75b7687.yaml new file mode 100644 index 000000000000..71a79556a4c8 --- /dev/null +++ b/releasenotes/notes/introduce-classical-io-channel-0a616e6ca75b7687.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + In the qiskit.pulse.channels package, ClassicalIOChannel class has been added + as an abstract base class of MemorySlot, RegisterSlot, and SnapshotChannel. + + The qiskit.pulse.transforms.canonicalization.pad method does not introduce + delays to any channels which are instances of ClassicalIOChannel. + + In qiskit.pulse.instructions, the constructors to SetPhase, ShiftPhase, + SetFrequency and ShiftFrequency now throw a PulseError if the channel parameter + is not of type PulseChannel. + From 7abc2ca7cdd601fe80e2e0ef96050036c8e054ae Mon Sep 17 00:00:00 2001 From: pollyshaw Date: Mon, 18 Jul 2022 23:31:43 +0100 Subject: [PATCH 13/13] Fixes #7078: remove typo --- qiskit/pulse/instructions/delay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/pulse/instructions/delay.py b/qiskit/pulse/instructions/delay.py index 1b911c350731..f045a4880ff3 100644 --- a/qiskit/pulse/instructions/delay.py +++ b/qiskit/pulse/instructions/delay.py @@ -47,7 +47,7 @@ def __init__( Args: duration: Length of time of the delay in terms of dt. channel: The channel that will have the delay. - name: Name of the delay for display purposes.k + name: Name of the delay for display purposes. """ super().__init__(operands=(duration, channel), name=name)