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

Remove complex amp support for ScalableSymbolicPulse #11403

Merged
merged 11 commits into from
Jan 15, 2024
38 changes: 9 additions & 29 deletions qiskit/pulse/library/symbolic_pulses.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
from qiskit.pulse.exceptions import PulseError
from qiskit.pulse.library.pulse import Pulse
from qiskit.pulse.library.waveform import Waveform
from qiskit.utils.deprecation import deprecate_arg


def _lifted_gaussian(
Expand Down Expand Up @@ -594,19 +593,6 @@ class ScalableSymbolicPulse(SymbolicPulse):
:math:'\text{amp}\times\exp\left(i\times\text{angle}\right)' is compared.
"""

@deprecate_arg(
"amp",
deprecation_description=(
"Setting ``amp`` to a complex in the ScalableSymbolicPulse constructor"
),
additional_msg=(
"Instead, use a float for ``amp`` (for the magnitude) and a float for ``angle``"
),
since="0.25.0",
package_name="qiskit-terra",
pending=False,
predicate=lambda amp: isinstance(amp, complex),
)
def __init__(
self,
pulse_type: str,
Expand Down Expand Up @@ -640,14 +626,12 @@ def __init__(
creates a full-waveform.

Raises:
PulseError: If both `amp` is complex and `angle` is not `None` or 0.
PulseError: If ``amp`` is complex.
"""
# This should be removed once complex amp support is removed.
if isinstance(amp, complex) and angle is not None and angle != 0:
raise PulseError("amp can't be complex with angle not None or 0")

if angle is None:
angle = 0
if isinstance(amp, complex):
raise PulseError(
"amp represents the magnitude of the complex amplitude and can't be complex"
)

if not isinstance(parameters, Dict):
parameters = {"amp": amp, "angle": angle}
Expand Down Expand Up @@ -749,7 +733,7 @@ def __new__(
duration: Union[int, ParameterExpression],
amp: ParameterValueType,
sigma: ParameterValueType,
angle: Optional[ParameterValueType] = None,
angle: Optional[ParameterValueType] = 0.0,
name: Optional[str] = None,
limit_amplitude: Optional[bool] = None,
) -> ScalableSymbolicPulse:
Expand All @@ -758,7 +742,6 @@ def __new__(
Args:
duration: Pulse length in terms of the sampling period `dt`.
amp: The magnitude of the amplitude of the Gaussian envelope.
Complex amp support is deprecated.
sigma: A measure of how wide or narrow the Gaussian peak is; described mathematically
in the class docstring.
angle: The angle of the complex amplitude of the Gaussian envelope. Default value 0.
Expand Down Expand Up @@ -843,7 +826,7 @@ def __new__(
amp: ParameterValueType,
sigma: ParameterValueType,
width: Optional[ParameterValueType] = None,
angle: Optional[ParameterValueType] = None,
angle: Optional[ParameterValueType] = 0.0,
risefall_sigma_ratio: Optional[ParameterValueType] = None,
name: Optional[str] = None,
limit_amplitude: Optional[bool] = None,
Expand All @@ -853,7 +836,6 @@ def __new__(
Args:
duration: Pulse length in terms of the sampling period `dt`.
amp: The magnitude of the amplitude of the Gaussian and square pulse.
Complex amp support is deprecated.
sigma: A measure of how wide or narrow the Gaussian risefall is; see the class
docstring for more details.
width: The duration of the embedded square pulse.
Expand Down Expand Up @@ -1375,7 +1357,7 @@ def __new__(
amp: ParameterValueType,
sigma: ParameterValueType,
beta: ParameterValueType,
angle: Optional[ParameterValueType] = None,
angle: Optional[ParameterValueType] = 0.0,
name: Optional[str] = None,
limit_amplitude: Optional[bool] = None,
) -> ScalableSymbolicPulse:
Expand All @@ -1384,7 +1366,6 @@ def __new__(
Args:
duration: Pulse length in terms of the sampling period `dt`.
amp: The magnitude of the amplitude of the DRAG envelope.
Complex amp support is deprecated.
sigma: A measure of how wide or narrow the Gaussian peak is; described mathematically
in the class docstring.
beta: The correction amplitude.
Expand Down Expand Up @@ -1441,7 +1422,7 @@ def __new__(
cls,
duration: Union[int, ParameterExpression],
amp: ParameterValueType,
angle: Optional[ParameterValueType] = None,
angle: Optional[ParameterValueType] = 0.0,
name: Optional[str] = None,
limit_amplitude: Optional[bool] = None,
) -> ScalableSymbolicPulse:
Expand All @@ -1450,7 +1431,6 @@ def __new__(
Args:
duration: Pulse length in terms of the sampling period `dt`.
amp: The magnitude of the amplitude of the square envelope.
Complex amp support is deprecated.
angle: The angle of the complex amplitude of the square envelope. Default value 0.
name: Display name for this pulse envelope.
limit_amplitude: If ``True``, then limit the amplitude of the
Expand Down
30 changes: 19 additions & 11 deletions qiskit/qpy/binary_io/schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,15 @@ def _read_symbolic_pulse(file_obj, version):
class_name = "SymbolicPulse" # Default class name, if not in the library

if pulse_type in legacy_library_pulses:
# Once complex amp support will be deprecated we will need:
# parameters["angle"] = np.angle(parameters["amp"])
# parameters["amp"] = np.abs(parameters["amp"])

# In the meanwhile we simply add:
parameters["angle"] = 0
parameters["angle"] = np.angle(parameters["amp"])
parameters["amp"] = np.abs(parameters["amp"])
_amp, _angle = sym.symbols("amp, angle")
envelope = envelope.subs(_amp, _amp * sym.exp(sym.I * _angle))

# And warn that this will change in future releases:
warnings.warn(
"Complex amp support for symbolic library pulses will be deprecated. "
"Once deprecated, library pulses loaded from old QPY files (Terra version < 0.23),"
" will be converted automatically to float (amp,angle) representation.",
QPYLoadingDeprecatedFeatureWarning,
f"Library pulses with complex amp are no longer supported."
TsafrirA marked this conversation as resolved.
Show resolved Hide resolved
f"{pulse_type} with complex amp was converted to (amp,angle) representation.",
UserWarning,
)
class_name = "ScalableSymbolicPulse"

Expand Down Expand Up @@ -237,6 +231,20 @@ def _read_symbolic_pulse_v6(file_obj, version, use_symengine):
valid_amp_conditions=valid_amp_conditions,
)
elif class_name == "ScalableSymbolicPulse":
# Between Qiskit 0.40 and 0.46, the (amp, angle) representation was present,
# but complex amp was still allowed. In Qiskit 1.0 and beyond complex amp
# is no longer supported and so the amp needs to be checked and converted.
# Once QPY version is bumped, a new reader function can be introduced without
# this check.
if isinstance(parameters["amp"], complex):
parameters["angle"] = np.angle(parameters["amp"])
parameters["amp"] = np.abs(parameters["amp"])
warnings.warn(
f"ScalableSymbolicPulse with complex amp are no longer supported."
TsafrirA marked this conversation as resolved.
Show resolved Hide resolved
f"{pulse_type} with complex amp was converted to (amp,angle) representation.",
UserWarning,
)

return library.ScalableSymbolicPulse(
pulse_type=pulse_type,
duration=duration,
Expand Down
10 changes: 2 additions & 8 deletions test/python/pulse/test_parameter_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,7 @@ def test_complex_valued_parameter(self):
with self.assertWarns(PendingDeprecationWarning):
assigned = visitor.visit(test_obj)

with self.assertWarns(DeprecationWarning):
ref_obj = pulse.Constant(duration=160, amp=1j * 0.1)

self.assertEqual(assigned, ref_obj)
self.assertEqual(assigned.amp, 0.1j)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this is confusing because we can still use complex valued amplitude through parameterization (because this bypasses validation in the constructor). We can probably drop validation in the constructor and let user use valid amplitude value on their responsibility. Note that this pulse will fail in execution anyways because QPY doesn't take complex value, right? I think this test doesn't really make sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nkanazawa1989

With Qiskit's main branch you can save a ScheduleBlock with complex amp.

In #9897 we added a PendingDeprecationWarning for the assignment of complex values. We can promote to deprecation in 0.46 and remove in 1.0 if you want.


def test_complex_value_to_parameter(self):
"""Test complex value can be assigned to parameter object,
Expand All @@ -370,10 +367,7 @@ def test_complex_value_to_parameter(self):
with self.assertWarns(PendingDeprecationWarning):
assigned = visitor.visit(test_obj)

with self.assertWarns(DeprecationWarning):
ref_obj = pulse.Constant(duration=160, amp=1j * 0.1)

self.assertEqual(assigned, ref_obj)
self.assertEqual(assigned.amp, 0.1j)

def test_complex_parameter_expression(self):
"""Test assignment of complex-valued parameter expression to parameter,
Expand Down
36 changes: 8 additions & 28 deletions test/python/pulse/test_pulse_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from qiskit.circuit import Parameter
from qiskit.pulse.library import (
SymbolicPulse,
ScalableSymbolicPulse,
Waveform,
Constant,
Gaussian,
Expand Down Expand Up @@ -147,32 +148,6 @@ def test_construction(self):
Sech(duration=50, amp=0.5, sigma=10, zero_ends=False)
SechDeriv(duration=50, amp=0.5, sigma=10)

# This test should be removed once deprecation of complex amp is completed.
def test_complex_amp_deprecation(self):
"""Test that deprecation warnings and errors are raised for complex amp,
and that pulses are equivalent."""

# Test deprecation warnings and errors:
with self.assertWarns(DeprecationWarning):
Gaussian(duration=25, sigma=4, amp=0.5j)
with self.assertWarns(DeprecationWarning):
GaussianSquare(duration=125, sigma=4, amp=0.5j, width=100)
with self.assertWarns(DeprecationWarning):
with self.assertRaises(PulseError):
Gaussian(duration=25, sigma=4, amp=0.5j, angle=1)
with self.assertWarns(DeprecationWarning):
with self.assertRaises(PulseError):
GaussianSquare(duration=125, sigma=4, amp=0.5j, width=100, angle=0.1)

# Test that new and old API pulses are the same:
with self.assertWarns(DeprecationWarning):
gauss_pulse_complex_amp = Gaussian(duration=25, sigma=4, amp=0.5j)
gauss_pulse_amp_angle = Gaussian(duration=25, sigma=4, amp=0.5, angle=np.pi / 2)
np.testing.assert_almost_equal(
gauss_pulse_amp_angle.get_waveform().samples,
gauss_pulse_complex_amp.get_waveform().samples,
)

def test_gauss_square_extremes(self):
"""Test that the gaussian square pulse can build a gaussian."""
duration = 125
Expand Down Expand Up @@ -498,7 +473,7 @@ def test_repr(self):
self.assertEqual(repr(gaus), "Gaussian(duration=25, sigma=4, amp=0.7, angle=0.3)")
gaus_square = GaussianSquare(duration=20, sigma=30, amp=1.0, width=3)
self.assertEqual(
repr(gaus_square), "GaussianSquare(duration=20, sigma=30, width=3, amp=1.0, angle=0)"
repr(gaus_square), "GaussianSquare(duration=20, sigma=30, width=3, amp=1.0, angle=0.0)"
)
gaus_square = GaussianSquare(
duration=20, sigma=30, amp=1.0, angle=0.2, risefall_sigma_ratio=0.1
Expand Down Expand Up @@ -534,7 +509,7 @@ def test_repr(self):
),
)
drag = Drag(duration=5, amp=0.5, sigma=7, beta=1)
self.assertEqual(repr(drag), "Drag(duration=5, sigma=7, beta=1, amp=0.5, angle=0)")
self.assertEqual(repr(drag), "Drag(duration=5, sigma=7, beta=1, amp=0.5, angle=0.0)")
const = Constant(duration=150, amp=0.1, angle=0.3)
self.assertEqual(repr(const), "Constant(duration=150, amp=0.1, angle=0.3)")
sin_pulse = Sin(duration=150, amp=0.1, angle=0.3, freq=0.2, phase=0)
Expand Down Expand Up @@ -920,6 +895,11 @@ def test_scalable_comparison(self):
gaussian1._params["sigma"] = 10
self.assertNotEqual(gaussian1, gaussian2)

def test_complex_amp_error(self):
"""Test that initializing a pulse with complex amp raises an error"""
with self.assertRaises(PulseError):
ScalableSymbolicPulse("test", duration=100, amp=0.1j, angle=0.0)


if __name__ == "__main__":
unittest.main()
21 changes: 19 additions & 2 deletions test/qpy_compat/test_qpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from qiskit.quantum_info import Operator
from qiskit.circuit.library import U1Gate, U2Gate, U3Gate, QFT, DCXGate, PauliGate
from qiskit.circuit.gate import Gate
from qiskit.version import VERSION as current_version_str

try:
from qiskit.qpy import dump, load
Expand Down Expand Up @@ -428,13 +429,18 @@ def generate_schedule_blocks():
from qiskit.pulse import builder, channels, library
from qiskit.utils import optionals

current_version = current_version_str.split(".")
for i in range(len(current_version[2])):
if current_version[2][i].isalpha():
current_version[2] = current_version[2][:i]
break
current_version = tuple(int(x) for x in current_version)
# Parameterized schedule test is avoided.
# Generated reference and loaded QPY object may induce parameter uuid mismatch.
# As workaround, we need test with bounded parameters, however, schedule.parameters
# are returned as Set and thus its order is random.
# Since schedule parameters are validated, we cannot assign random numbers.
# We need to upgrade testing framework.

schedule_blocks = []

# Instructions without parameters
Expand All @@ -445,7 +451,18 @@ def generate_schedule_blocks():
builder.set_phase(1.57, channels.DriveChannel(0))
builder.shift_phase(0.1, channels.DriveChannel(1))
builder.barrier(channels.DriveChannel(0), channels.DriveChannel(1))
builder.play(library.Gaussian(160, 0.1j, 40), channels.DriveChannel(0))
gaussian_amp = 0.1
gaussian_angle = 0.7
if current_version < (1, 0, 0):
builder.play(
library.Gaussian(160, gaussian_amp * np.exp(1j * gaussian_angle), 40),
channels.DriveChannel(0),
)
else:
builder.play(
library.Gaussian(160, gaussian_amp, 40, gaussian_angle),
channels.DriveChannel(0),
)
builder.play(library.GaussianSquare(800, 0.1, 64, 544), channels.ControlChannel(0))
builder.play(library.Drag(160, 0.1, 40, 1.5), channels.DriveChannel(1))
builder.play(library.Constant(800, 0.1), channels.MeasureChannel(0))
Expand Down
Loading