From 4b38e906c5f6e290b666231f5879daa4fd65099b Mon Sep 17 00:00:00 2001 From: Will Shanks Date: Wed, 31 Jan 2024 12:39:22 -0500 Subject: [PATCH 1/4] Deprecate pulse reference parameter scoping As noted in https://github.com/Qiskit/qiskit/issues/11654, parameter scoping does not work properly beginning with Qiskit 0.45.0. The problem is that the `Parameter` name is now used for the hash value and so the parameter produced with the scoped name is not equivalent to the original parameter and does not substitute for it when using parameter assignment. Since the scoped parameter mechanism was a convenience feature and not widely used, it is simply deprecated rather than made to work with parameter assignment. --- qiskit/pulse/schedule.py | 25 ++++++ ...se-parameter-scoping-6b6b50a394b57937.yaml | 11 +++ test/python/pulse/test_reference.py | 81 +++++++++++-------- 3 files changed, 85 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml diff --git a/qiskit/pulse/schedule.py b/qiskit/pulse/schedule.py index a1f7ced82d8b..923760b43dfa 100644 --- a/qiskit/pulse/schedule.py +++ b/qiskit/pulse/schedule.py @@ -51,6 +51,7 @@ from qiskit.pulse.instructions import Instruction, Reference from qiskit.pulse.utils import instruction_duration_validation from qiskit.pulse.reference_manager import ReferenceManager +from qiskit.utils.deprecation import deprecate_func from qiskit.utils.multiprocessing import is_main_process @@ -880,6 +881,12 @@ class ScheduleBlock: .. rubric:: Program Scoping + .. note:: + + The :meth:`~ScheduleBlock.scoped_parameters` and + :meth:`~ScheduleBlock.search_parameters` methods described in this + section are deprecated. + When you call a subroutine from another subroutine, or append a schedule block to another schedule block, the management of references and parameters can be a hard task. Schedule block offers a convenient feature to help with this @@ -1225,6 +1232,13 @@ def parameters(self) -> Set[Parameter]: return out_params + @deprecate_func( + additional_msg=( + "There is no alternative to this method. Parameters must be mapped " + "to references by checking the reference schedules directly." + ), + since="0.46.0", + ) def scoped_parameters(self) -> Tuple[Parameter]: """Return unassigned parameters with scoped names. @@ -1623,6 +1637,13 @@ def get_parameters(self, parameter_name: str) -> List[Parameter]: matched = [p for p in self.parameters if p.name == parameter_name] return matched + @deprecate_func( + additional_msg=( + "There is no alternative to this method. Parameters must be mapped " + "to references by checking the reference schedules directly." + ), + since="0.46.0", + ) def search_parameters(self, parameter_regex: str) -> List[Parameter]: """Search parameter with regular expression. @@ -1963,6 +1984,10 @@ def _collect_scoped_parameters( Returns: A dictionary of scoped parameter objects. """ + warnings.warn( + "Scoped parameters may not work correctly with parameter assignment.", + stacklevel=3 + ) parameters_out = {} for param in schedule._parameter_manager.parameters: new_name = f"{current_scope}{Reference.scope_delimiter}{param.name}" diff --git a/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml b/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml new file mode 100644 index 000000000000..dd887c6dda65 --- /dev/null +++ b/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml @@ -0,0 +1,11 @@ +--- +deprecations: + - | + The :meth:`.ScheduleBlock.scoped_parameters` and + :meth:`.ScheduleBlock.search_parameters` methods have been deprecated. + These methods produce :class:`.Parameter` objects with names modified to + indicate pulse scoping. The original intention of the methods was that + these objects would still link to the original unscoped :class:`Parameter` + objects. However, the modification of the name breaks the link so that + assigning using the scoped version does not work. See `#11654 + `__ for more context. diff --git a/test/python/pulse/test_reference.py b/test/python/pulse/test_reference.py index 0996ecb3be31..86fa9940d53b 100644 --- a/test/python/pulse/test_reference.py +++ b/test/python/pulse/test_reference.py @@ -56,14 +56,16 @@ def test_append_schedule_parameter_scope(self): with pulse.build() as sched_z1: builder.append_schedule(sched_y1) - sched_param = next(iter(sched_z1.scoped_parameters())) + with self.assertWarns(DeprecationWarning): + sched_param = next(iter(sched_z1.scoped_parameters())) self.assertEqual(sched_param.name, "root::name") # object equality - self.assertEqual( - sched_z1.search_parameters("root::name")[0], - param, - ) + with self.assertWarns(DeprecationWarning): + self.assertEqual( + sched_z1.search_parameters("root::name")[0], + param, + ) def test_refer_schedule(self): """Test refer to schedule by name. @@ -108,20 +110,23 @@ def test_refer_schedule_parameter_scope(self): sched_y1.assign_references({("x1", "d0"): sched_x1}) sched_z1.assign_references({("y1", "d0"): sched_y1}) - sched_param = next(iter(sched_z1.scoped_parameters())) + with self.assertWarns(DeprecationWarning): + sched_param = next(iter(sched_z1.scoped_parameters())) self.assertEqual(sched_param.name, "root::y1,d0::x1,d0::name") # object equality - self.assertEqual( - sched_z1.search_parameters("root::y1,d0::x1,d0::name")[0], - param, - ) + with self.assertWarns(DeprecationWarning): + self.assertEqual( + sched_z1.search_parameters("root::y1,d0::x1,d0::name")[0], + param, + ) # regex - self.assertEqual( - sched_z1.search_parameters(r"\S::x1,d0::name")[0], - param, - ) + with self.assertWarns(DeprecationWarning): + self.assertEqual( + sched_z1.search_parameters(r"\S::x1,d0::name")[0], + param, + ) def test_call_schedule(self): """Test call schedule. @@ -160,20 +165,23 @@ def test_call_schedule_parameter_scope(self): with pulse.build() as sched_z1: builder.call(sched_y1, name="y1") - sched_param = next(iter(sched_z1.scoped_parameters())) + with self.assertWarns(DeprecationWarning): + sched_param = next(iter(sched_z1.scoped_parameters())) self.assertEqual(sched_param.name, "root::y1::x1::name") # object equality - self.assertEqual( - sched_z1.search_parameters("root::y1::x1::name")[0], - param, - ) + with self.assertWarns(DeprecationWarning): + self.assertEqual( + sched_z1.search_parameters("root::y1::x1::name")[0], + param, + ) # regex - self.assertEqual( - sched_z1.search_parameters(r"\S::x1::name")[0], - param, - ) + with self.assertWarns(DeprecationWarning): + self.assertEqual( + sched_z1.search_parameters(r"\S::x1::name")[0], + param, + ) def test_append_and_call_schedule(self): """Test call and append schedule. @@ -368,7 +376,8 @@ def test_special_parameter_name(self): pulse.reference("sub", "q0") sched_y1.assign_references({("sub", "q0"): sched_x1}) - ret_param = sched_y1.search_parameters(r"\Ssub,q0::my.parameter_object")[0] + with self.assertWarns(DeprecationWarning): + ret_param = sched_y1.search_parameters(r"\Ssub,q0::my.parameter_object")[0] self.assertEqual(param, ret_param) @@ -392,10 +401,13 @@ def test_parameter_in_multiple_scope(self): pulse.call(sched_y1, name="y1") self.assertEqual(len(sched_z1.parameters), 1) - self.assertEqual(len(sched_z1.scoped_parameters()), 2) + with self.assertWarns(DeprecationWarning): + self.assertEqual(len(sched_z1.scoped_parameters()), 2) - self.assertEqual(sched_z1.search_parameters("root::x1::name")[0], param) - self.assertEqual(sched_z1.search_parameters("root::y1::name")[0], param) + with self.assertWarns(DeprecationWarning): + self.assertEqual(sched_z1.search_parameters("root::x1::name")[0], param) + with self.assertWarns(DeprecationWarning): + self.assertEqual(sched_z1.search_parameters("root::y1::name")[0], param) def test_parallel_alignment_equality(self): """Testcase for potential edge case. @@ -558,7 +570,8 @@ def test_lazy_ecr(self): self.assertSetEqual(params, {"cr"}) # Parameter names are scoepd - scoped_params = {p.name for p in sched.scoped_parameters()} + with self.assertWarns(DeprecationWarning): + scoped_params = {p.name for p in sched.scoped_parameters()} self.assertSetEqual(scoped_params, {"root::cr"}) # Assign CR and XP schedule to the empty reference @@ -571,7 +584,8 @@ def test_lazy_ecr(self): self.assertEqual(assigned_refs[("xp", "q0")], self.xp_sched) # Parameter added from subroutines - scoped_params = {p.name for p in sched.scoped_parameters()} + with self.assertWarns(DeprecationWarning): + scoped_params = {p.name for p in sched.scoped_parameters()} ref_params = { # This is the cr parameter that belongs to phase_offset instruction in the root scope "root::cr", @@ -594,7 +608,8 @@ def test_lazy_ecr(self): self.assertEqual(len(params), 2) # Get parameter with scope, only xp amp - params = sched.search_parameters(parameter_regex="root::xp,q0::amp") + with self.assertWarns(DeprecationWarning): + params = sched.search_parameters(parameter_regex="root::xp,q0::amp") self.assertEqual(len(params), 1) def test_cnot(self): @@ -614,11 +629,13 @@ def test_cnot(self): pulse.call(ecr_sched, name="ecr") # get parameter with scope, full scope is not needed - xp_amp = cx_sched.search_parameters(r"\S:xp::amp")[0] + with self.assertWarns(DeprecationWarning): + xp_amp = cx_sched.search_parameters(r"\S:xp::amp")[0] self.assertEqual(self.xp_amp, xp_amp) # get parameter with scope, of course full scope can be specified - xp_amp_full_scoped = cx_sched.search_parameters("root::ecr::xp::amp")[0] + with self.assertWarns(DeprecationWarning): + xp_amp_full_scoped = cx_sched.search_parameters("root::ecr::xp::amp")[0] self.assertEqual(xp_amp_full_scoped, xp_amp) # assign parameters From 414ee6208d75222a2906e7fbdfe1f0f04082a451 Mon Sep 17 00:00:00 2001 From: Will Shanks Date: Wed, 31 Jan 2024 16:42:01 -0500 Subject: [PATCH 2/4] black --- qiskit/pulse/schedule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qiskit/pulse/schedule.py b/qiskit/pulse/schedule.py index 923760b43dfa..5bb82e840aa1 100644 --- a/qiskit/pulse/schedule.py +++ b/qiskit/pulse/schedule.py @@ -1986,7 +1986,7 @@ def _collect_scoped_parameters( """ warnings.warn( "Scoped parameters may not work correctly with parameter assignment.", - stacklevel=3 + stacklevel=3, ) parameters_out = {} for param in schedule._parameter_manager.parameters: From 3d7951b67fe005309df4f600323bbe9e5c1c8f13 Mon Sep 17 00:00:00 2001 From: Will Shanks Date: Wed, 31 Jan 2024 23:47:41 -0500 Subject: [PATCH 3/4] Update releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml Co-authored-by: Jake Lishman --- .../deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml b/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml index dd887c6dda65..69355a1ccc3c 100644 --- a/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml +++ b/releasenotes/notes/deprecate-pulse-parameter-scoping-6b6b50a394b57937.yaml @@ -5,7 +5,7 @@ deprecations: :meth:`.ScheduleBlock.search_parameters` methods have been deprecated. These methods produce :class:`.Parameter` objects with names modified to indicate pulse scoping. The original intention of the methods was that - these objects would still link to the original unscoped :class:`Parameter` + these objects would still link to the original unscoped :class:`.Parameter` objects. However, the modification of the name breaks the link so that assigning using the scoped version does not work. See `#11654 `__ for more context. From 7b55ad00ac08d763d2d14737f253b32603ee0b4b Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Thu, 1 Feb 2024 12:15:16 +0000 Subject: [PATCH 4/4] Move warning to documentation --- qiskit/pulse/schedule.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/qiskit/pulse/schedule.py b/qiskit/pulse/schedule.py index 5bb82e840aa1..7ccdabfe2449 100644 --- a/qiskit/pulse/schedule.py +++ b/qiskit/pulse/schedule.py @@ -1238,10 +1238,16 @@ def parameters(self) -> Set[Parameter]: "to references by checking the reference schedules directly." ), since="0.46.0", + removal_timeline="in the Qiskit 1.0 release", ) def scoped_parameters(self) -> Tuple[Parameter]: """Return unassigned parameters with scoped names. + .. warning:: + + Scoped parameters do not work correctly with Qiskit's data model for parameter + assignment. This implementation is consequently being removed in Qiskit 1.0. + .. note:: If a parameter is defined within a nested scope, @@ -1643,10 +1649,16 @@ def get_parameters(self, parameter_name: str) -> List[Parameter]: "to references by checking the reference schedules directly." ), since="0.46.0", + removal_timeline="in the Qiskit 1.0 release", ) def search_parameters(self, parameter_regex: str) -> List[Parameter]: """Search parameter with regular expression. + .. warning:: + + Scoped parameters do not work correctly with Qiskit's data model for parameter + assignment. This implementation is consequently being removed in Qiskit 1.0. + This method looks for the scope-aware parameters. For example, @@ -1984,10 +1996,6 @@ def _collect_scoped_parameters( Returns: A dictionary of scoped parameter objects. """ - warnings.warn( - "Scoped parameters may not work correctly with parameter assignment.", - stacklevel=3, - ) parameters_out = {} for param in schedule._parameter_manager.parameters: new_name = f"{current_scope}{Reference.scope_delimiter}{param.name}"