-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Serializable parametric pulse #7821
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
Changes from all commits
afe4f96
9a8a582
6f2496a
0424636
1bf11fa
2335f32
2af235d
0cc7383
a59af77
21ef762
509470a
ed2f661
c4b41d5
71ef841
cf7f302
c2981d0
cf17341
ba685ef
f2ef950
2ea6390
f48796e
0738497
92f42eb
1440a23
4345c3b
1c63ddb
d121b0d
48ff3cc
b9cab67
42b3dd9
761235d
cc12bf8
6c21edd
43e0e0b
6b956de
eefa5e4
252f62b
f24e648
4d191be
2c8e7f8
3a0f506
d5517d5
be2ed0c
67ac61f
78288ac
cbada64
ecb4ea7
45ae755
a4a56db
d957458
6776743
d4e8c43
41bf4f5
2aa4f5a
2e521a7
09855d7
b99eb5c
ac23d6f
401c1f4
686e77b
84344c2
914018e
045d67b
3b2c571
8edaba9
9b0adc3
cda7336
ef35cd9
15e9f7c
d844253
526aaed
87d3538
be2b4e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ class ParametricPulseShapes(Enum): | |
| ... | ||
| new_supported_pulse_name = library.YourPulseWaveformClass | ||
| """ | ||
| import warnings | ||
| from abc import abstractmethod | ||
| from typing import Any, Dict, Optional, Union | ||
|
|
||
|
|
@@ -51,7 +52,15 @@ class ParametricPulseShapes(Enum): | |
|
|
||
|
|
||
| class ParametricPulse(Pulse): | ||
| """The abstract superclass for parametric pulses.""" | ||
| """The abstract superclass for parametric pulses. | ||
|
|
||
| .. warning:: | ||
|
|
||
| This class is superseded by :class:`.SymbolicPulse` and will be deprecated | ||
| and eventually removed in the future because of the poor flexibility | ||
| for defining a new waveform type and serializing it through the :mod:`qiskit.qpy` framework. | ||
|
|
||
| """ | ||
|
|
||
| @abstractmethod | ||
| def __init__( | ||
|
|
@@ -70,6 +79,14 @@ def __init__( | |
| amplitude is constrained to 1. | ||
| """ | ||
| super().__init__(duration=duration, name=name, limit_amplitude=limit_amplitude) | ||
|
|
||
|
nkanazawa1989 marked this conversation as resolved.
nkanazawa1989 marked this conversation as resolved.
|
||
| warnings.warn( | ||
| "ParametricPulse and its subclass will be deprecated and will be replaced with " | ||
| "SymbolicPulse and its subclass because of QPY serialization support. " | ||
| "See qiskit.pulse.library.symbolic_pulses for details.", | ||
| PendingDeprecationWarning, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replacing
What I am a little concerned about is this disconnect between fully specifying a pulse with a symbolic expression and specifying a pulse as a name and set of parameters that is implemented by the backend. Is it best to try to blur the line between those two or keep them explicitly two separate things? Serialization is not in this PR, but another thing I wonder about is if Gaussian, for example, would be serialized as a symbolic expression and deserialized as a generic symbolic pulse or if it would be serialized as a "gaussian" pulse and then deserialized back to a Gaussian.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Data format in the communication layer is one of the important aspects of the symbolic pulse, however, for us, the most important feature is the serialization of calibration database, where users may actively introduce new pulse shapes with new calibration techniques. I tend to disagree with supporting both
This is why we need symbolic pulse. Indeed how this code is used is unclear now, because job submission mechanism is totally offloaded to provider. Qiskit just passes python object to the provider. If one introduces new pulse shape in the backend config, he/she should update the provider code together with parametric pulse library in Qiskit. And user need to wait for next release to enjoy backend-supported parametric pulse. I think this is why DCX (target) pulse is still provided as waveform. I agree keeping current mechanism of name + parameters for predefined pulses. This will help us to reduce data volume of QPY binary. Regarding the serialization, I plan to write symbolic equations in the program header, instead of duplicating it for every appearance of play instruction. For example, in the circuit we have If we only save equations of custom pulses, we can compactly represent the program (i.e. DRAG is very often used but constraints is quite big object). Same mechanism exists in circuit IO.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The backend configuration includes a Here is a suggestion for how things could work differently to decouple the provider from terra:
Then a provider could add support for additional pulses and the user would just need to define
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was thinking to use If the symbolic pulse needs to use conventional names in which may confuse the users. This also changes labels in the pulse visualizer and
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, so what is a path forward for not requiring pulses to be hardcoded in terra for them to be passed parametrically to the backend? Wait for a backend and provider that supports QPY jobs and arbitrary symbolic pulses? For backwards compatibility we could store I don't see a way to avoid keeping the enum defined for now for the instruction schedule map usage. Perhaps we could get backends to start publishing the CmdDef as |
||
| stacklevel=3, | ||
| ) | ||
| self.validate_parameters() | ||
|
|
||
| @abstractmethod | ||
|
|
@@ -155,7 +172,7 @@ def get_waveform(self) -> Waveform: | |
| return gaussian(duration=self.duration, amp=self.amp, sigma=self.sigma, zero_ends=True) | ||
|
|
||
| def validate_parameters(self) -> None: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self.limit_amplitude: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self._limit_amplitude: | ||
| raise PulseError( | ||
| f"The amplitude norm must be <= 1, found: {abs(self.amp)}" | ||
| + "This can be overruled by setting Pulse.limit_amplitude." | ||
|
|
@@ -287,7 +304,7 @@ def get_waveform(self) -> Waveform: | |
| ) | ||
|
|
||
| def validate_parameters(self) -> None: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self.limit_amplitude: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self._limit_amplitude: | ||
| raise PulseError( | ||
| f"The amplitude norm must be <= 1, found: {abs(self.amp)}" | ||
| + "This can be overruled by setting Pulse.limit_amplitude." | ||
|
|
@@ -431,7 +448,7 @@ def get_waveform(self) -> Waveform: | |
| ) | ||
|
|
||
| def validate_parameters(self) -> None: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self.limit_amplitude: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self._limit_amplitude: | ||
| raise PulseError( | ||
| f"The amplitude norm must be <= 1, found: {abs(self.amp)}" | ||
| + "This can be overruled by setting Pulse.limit_amplitude." | ||
|
|
@@ -445,7 +462,7 @@ def validate_parameters(self) -> None: | |
| not _is_parameterized(self.beta) | ||
| and not _is_parameterized(self.sigma) | ||
| and np.abs(self.beta) > self.sigma | ||
| and self.limit_amplitude | ||
| and self._limit_amplitude | ||
| ): | ||
| # If beta <= sigma, then the maximum amplitude is at duration / 2, which is | ||
| # already constrained by self.amp <= 1 | ||
|
|
@@ -528,7 +545,7 @@ def get_waveform(self) -> Waveform: | |
| return constant(duration=self.duration, amp=self.amp) | ||
|
|
||
| def validate_parameters(self) -> None: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self.limit_amplitude: | ||
| if not _is_parameterized(self.amp) and abs(self.amp) > 1.0 and self._limit_amplitude: | ||
| raise PulseError( | ||
| f"The amplitude norm must be <= 1, found: {abs(self.amp)}" | ||
| + "This can be overruled by setting Pulse.limit_amplitude." | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.