From b553b9e81a7c3bea5b29520389183f65bf34c527 Mon Sep 17 00:00:00 2001 From: TsafrirA Date: Sun, 5 Mar 2023 23:58:46 +0200 Subject: [PATCH 1/5] Add warning and update tests --- qiskit/pulse/parameter_manager.py | 12 +++++++++ test/python/pulse/test_parameter_manager.py | 29 +++++++++++++-------- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/qiskit/pulse/parameter_manager.py b/qiskit/pulse/parameter_manager.py index d1db89ab1bed..4b46f26c4693 100644 --- a/qiskit/pulse/parameter_manager.py +++ b/qiskit/pulse/parameter_manager.py @@ -52,6 +52,7 @@ """ from copy import copy from typing import List, Dict, Set, Any, Union +import warnings from qiskit.circuit.parameter import Parameter from qiskit.circuit.parameterexpression import ParameterExpression, ParameterValueType @@ -231,6 +232,17 @@ def visit_SymbolicPulse(self, node: SymbolicPulse): pval = node._params[name] if isinstance(pval, ParameterExpression): new_val = self._assign_parameter_expression(pval) + if isinstance(new_val, complex): + warnings.warn( + "Assignment of complex parameters to SymbolicPulse is now pending " + "deprecation. As of Qiskit-Terra 0.23.0 all library pulses were " + "converted from complex amplitude representation to real representation " + "using two floats (amp,angle), as used in the ScalableSymbolicPulse " + "class. This eliminated the need for complex parameters. Any custom-built " + "pulses should be converted in a similar fashion to avoid the use of " + "complex parameters.", + PendingDeprecationWarning, + ) node._params[name] = new_val node.validate_parameters() diff --git a/test/python/pulse/test_parameter_manager.py b/test/python/pulse/test_parameter_manager.py index bd0cacf6bd21..4ffa519c0a8a 100644 --- a/test/python/pulse/test_parameter_manager.py +++ b/test/python/pulse/test_parameter_manager.py @@ -201,7 +201,7 @@ class TestParameterSetter(ParameterTestBase): """Test setting parameters.""" def test_set_parameter_to_channel(self): - """Test get parameters from channel.""" + """Test set parameters from channel.""" test_obj = pulse.DriveChannel(self.ch1 + self.ch2) value_dict = {self.ch1: 1, self.ch2: 2} @@ -214,7 +214,7 @@ def test_set_parameter_to_channel(self): self.assertEqual(assigned, ref_obj) def test_set_parameter_to_pulse(self): - """Test get parameters from pulse instruction.""" + """Test set parameters from pulse instruction.""" test_obj = self.parametric_waveform1 value_dict = {self.amp1_1: 0.1, self.amp1_2: 0.2, self.dur1: 160} @@ -336,42 +336,47 @@ def test_nested_assignment_partial_bind(self): self.assertEqual(assigned, ref_obj) def test_complex_valued_parameter(self): - """Test complex valued parameter can be casted to a complex value.""" + """Test complex valued parameter can be casted to a complex value, + but raises PendingDeprecationWarning..""" amp = Parameter("amp") test_obj = pulse.Constant(duration=160, amp=1j * amp) value_dict = {amp: 0.1} visitor = ParameterSetter(param_map=value_dict) - assigned = visitor.visit(test_obj) + with self.assertWarns(PendingDeprecationWarning): + assigned = visitor.visit(test_obj) ref_obj = pulse.Constant(duration=160, amp=1j * 0.1) self.assertEqual(assigned, ref_obj) def test_complex_value_to_parameter(self): - """Test complex value can be assigned to parameter object.""" + """Test complex value can be assigned to parameter object, + but raises PendingDeprecationWarning.""" amp = Parameter("amp") test_obj = pulse.Constant(duration=160, amp=amp) value_dict = {amp: 0.1j} visitor = ParameterSetter(param_map=value_dict) - assigned = visitor.visit(test_obj) + with self.assertWarns(PendingDeprecationWarning): + assigned = visitor.visit(test_obj) ref_obj = pulse.Constant(duration=160, amp=1j * 0.1) self.assertEqual(assigned, ref_obj) def test_complex_parameter_expression(self): - """Test assignment of complex-valued parameter expression to parameter.""" + """Test assignment of complex-valued parameter expression to parameter, + but raises PendingDeprecationWarning.""" amp = Parameter("amp") mag = Parameter("A") phi = Parameter("phi") test_obj = pulse.Constant(duration=160, amp=amp) - + test_obj_copy = deepcopy(test_obj) # generate parameter expression value_dict = {amp: mag * np.exp(1j * phi)} visitor = ParameterSetter(param_map=value_dict) @@ -380,13 +385,15 @@ def test_complex_parameter_expression(self): # generate complex value value_dict = {mag: 0.1, phi: 0.5} visitor = ParameterSetter(param_map=value_dict) - assigned = visitor.visit(assigned) + with self.assertWarns(PendingDeprecationWarning): + assigned = visitor.visit(assigned) # evaluated parameter expression: 0.0877582561890373 + 0.0479425538604203*I value_dict = {amp: 0.1 * np.exp(0.5j)} - visitor = ParameterSetter(param_map=value_dict) - ref_obj = visitor.visit(test_obj) + visitor = ParameterSetter(param_map=value_dict) + with self.assertWarns(PendingDeprecationWarning): + ref_obj = visitor.visit(test_obj_copy) self.assertEqual(assigned, ref_obj) def test_invalid_pulse_amplitude(self): From f0e52562b6673f1c6e8099f8c5f577b4237d58b9 Mon Sep 17 00:00:00 2001 From: TsafrirA Date: Mon, 6 Mar 2023 23:14:17 +0200 Subject: [PATCH 2/5] Release notes --- ...bolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml diff --git a/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml new file mode 100644 index 000000000000..d5d212d14055 --- /dev/null +++ b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml @@ -0,0 +1,9 @@ +--- +deprecations: + - | + Assignment of complex values to `SymbolicPulse` instances now raises a + `PendingDeprecationWarning`. As of Qiskit-Terra 0.23.0 all library pulses + were converted from complex amplitude representation to real representation + using two floats (amp,angle), as used in the `ScalableSymbolicPulse` class. + This eliminated the need for complex parameters. Any custom-built pulses + should be converted in a similar fashion to avoid the use of complex parameters. From dde64de184089a9d78c45e963a52cd6eafdec0d0 Mon Sep 17 00:00:00 2001 From: TsafrirA Date: Wed, 15 Mar 2023 11:46:22 +0200 Subject: [PATCH 3/5] Move warning to all Pulse objects. --- qiskit/pulse/parameter_manager.py | 28 ++++++++++--------- ...-complex-deprecation-89ecdf968b1a2d89.yaml | 15 ++++++---- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/qiskit/pulse/parameter_manager.py b/qiskit/pulse/parameter_manager.py index 4b46f26c4693..0f99e17522d5 100644 --- a/qiskit/pulse/parameter_manager.py +++ b/qiskit/pulse/parameter_manager.py @@ -232,17 +232,6 @@ def visit_SymbolicPulse(self, node: SymbolicPulse): pval = node._params[name] if isinstance(pval, ParameterExpression): new_val = self._assign_parameter_expression(pval) - if isinstance(new_val, complex): - warnings.warn( - "Assignment of complex parameters to SymbolicPulse is now pending " - "deprecation. As of Qiskit-Terra 0.23.0 all library pulses were " - "converted from complex amplitude representation to real representation " - "using two floats (amp,angle), as used in the ScalableSymbolicPulse " - "class. This eliminated the need for complex parameters. Any custom-built " - "pulses should be converted in a similar fashion to avoid the use of " - "complex parameters.", - PendingDeprecationWarning, - ) node._params[name] = new_val node.validate_parameters() @@ -268,8 +257,21 @@ def _assign_parameter_expression(self, param_expr: ParameterExpression): updated = param_expr.parameters & self._param_map.keys() for param in updated: new_value = new_value.assign(param, self._param_map[param]) - - return format_parameter_value(new_value) + new_value = format_parameter_value(new_value) + if isinstance(new_value, complex): + warnings.warn( + "Assignment of complex values to ParameterExpression in Qiskit Pulse objects is " + "now pending deprecation. This will align the Pulse module with other modules " + "where such assignment wasn't possible to begin with. The typical use case for complex " + "parameters in the module was the SymbolicPulse library. As of Qiskit-Terra " + "0.23.0 all library pulses were converted from complex amplitude representation" + " to real representation using two floats (amp,angle), as used in the " + "ScalableSymbolicPulse class. This eliminated the need for complex parameters. " + "Any use of complex parameters (and particularly custom-built pulses) should be " + "converted in a similar fashion to avoid the use of complex parameters.", + PendingDeprecationWarning, + ) + return new_value def _update_parameter_manager(self, node: Union[Schedule, ScheduleBlock]): """A helper function to update parameter manager of pulse program.""" diff --git a/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml index d5d212d14055..027e73afd69d 100644 --- a/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml +++ b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml @@ -1,9 +1,12 @@ --- deprecations: - | - Assignment of complex values to `SymbolicPulse` instances now raises a - `PendingDeprecationWarning`. As of Qiskit-Terra 0.23.0 all library pulses - were converted from complex amplitude representation to real representation - using two floats (amp,angle), as used in the `ScalableSymbolicPulse` class. - This eliminated the need for complex parameters. Any custom-built pulses - should be converted in a similar fashion to avoid the use of complex parameters. + Assignment of complex values to ``ParameterExpression`` in any Qiskit Pulse object + now raises a ``PendingDeprecationWarning``. This will align the Pulse module with + other modules where such assignment wasn't possible to begin with. The typical use + case for complex parameters in the module was the SymbolicPulse library. As of + Qiskit-Terra 0.23.0 all library pulses were converted from complex amplitude + representation to real representation using two floats (amp,angle), as used in the + ``ScalableSymbolicPulse`` class. This eliminated the need for complex parameters. + Any use of complex parameters (and particularly custom-built pulses) should be + converted in a similar fashion to avoid the use of complex parameters." From 668c4edcd0dee7e9ca545b709683d80c0c06b834 Mon Sep 17 00:00:00 2001 From: TsafrirA Date: Wed, 15 Mar 2023 11:56:53 +0200 Subject: [PATCH 4/5] Fix releasenote typo --- .../symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml index 027e73afd69d..9f0377cf111c 100644 --- a/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml +++ b/releasenotes/notes/symbolic-pulse-complex-deprecation-89ecdf968b1a2d89.yaml @@ -9,4 +9,4 @@ deprecations: representation to real representation using two floats (amp,angle), as used in the ``ScalableSymbolicPulse`` class. This eliminated the need for complex parameters. Any use of complex parameters (and particularly custom-built pulses) should be - converted in a similar fashion to avoid the use of complex parameters." + converted in a similar fashion to avoid the use of complex parameters. From 313f311b3257bc87ecbf23a1269856d98b2f2e58 Mon Sep 17 00:00:00 2001 From: TsafrirA Date: Tue, 21 Mar 2023 01:11:51 +0200 Subject: [PATCH 5/5] Move warning to format_parameter_value --- qiskit/pulse/parameter_manager.py | 14 -------------- qiskit/pulse/utils.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/qiskit/pulse/parameter_manager.py b/qiskit/pulse/parameter_manager.py index 0f99e17522d5..4fac995b22f7 100644 --- a/qiskit/pulse/parameter_manager.py +++ b/qiskit/pulse/parameter_manager.py @@ -52,7 +52,6 @@ """ from copy import copy from typing import List, Dict, Set, Any, Union -import warnings from qiskit.circuit.parameter import Parameter from qiskit.circuit.parameterexpression import ParameterExpression, ParameterValueType @@ -258,19 +257,6 @@ def _assign_parameter_expression(self, param_expr: ParameterExpression): for param in updated: new_value = new_value.assign(param, self._param_map[param]) new_value = format_parameter_value(new_value) - if isinstance(new_value, complex): - warnings.warn( - "Assignment of complex values to ParameterExpression in Qiskit Pulse objects is " - "now pending deprecation. This will align the Pulse module with other modules " - "where such assignment wasn't possible to begin with. The typical use case for complex " - "parameters in the module was the SymbolicPulse library. As of Qiskit-Terra " - "0.23.0 all library pulses were converted from complex amplitude representation" - " to real representation using two floats (amp,angle), as used in the " - "ScalableSymbolicPulse class. This eliminated the need for complex parameters. " - "Any use of complex parameters (and particularly custom-built pulses) should be " - "converted in a similar fashion to avoid the use of complex parameters.", - PendingDeprecationWarning, - ) return new_value def _update_parameter_manager(self, node: Union[Schedule, ScheduleBlock]): diff --git a/qiskit/pulse/utils.py b/qiskit/pulse/utils.py index ee3d6b6e039a..03c215284842 100644 --- a/qiskit/pulse/utils.py +++ b/qiskit/pulse/utils.py @@ -13,6 +13,7 @@ """Module for common pulse programming utilities.""" import functools from typing import List, Dict, Union +import warnings import numpy as np @@ -65,6 +66,19 @@ def format_parameter_value( evaluated = float(evaluated.real) if evaluated.is_integer(): evaluated = int(evaluated) + else: + warnings.warn( + "Assignment of complex values to ParameterExpression in Qiskit Pulse objects is " + "now pending deprecation. This will align the Pulse module with other modules " + "where such assignment wasn't possible to begin with. The typical use case for complex " + "parameters in the module was the SymbolicPulse library. As of Qiskit-Terra " + "0.23.0 all library pulses were converted from complex amplitude representation" + " to real representation using two floats (amp,angle), as used in the " + "ScalableSymbolicPulse class. This eliminated the need for complex parameters. " + "Any use of complex parameters (and particularly custom-built pulses) should be " + "converted in a similar fashion to avoid the use of complex parameters.", + PendingDeprecationWarning, + ) return evaluated except TypeError: