From 4b6d6931498e3e56269425207b9b51d52448a459 Mon Sep 17 00:00:00 2001 From: knzwnao Date: Mon, 15 Nov 2021 18:01:44 +0900 Subject: [PATCH 1/2] fix format logic --- qiskit/pulse/utils.py | 24 ++++----- test/python/pulse/test_parameter_manager.py | 56 +++++++++++++++++++++ test/python/pulse/test_parameters.py | 15 +----- 3 files changed, 68 insertions(+), 27 deletions(-) diff --git a/qiskit/pulse/utils.py b/qiskit/pulse/utils.py index 4efb77d5136b..ba24fb661a50 100644 --- a/qiskit/pulse/utils.py +++ b/qiskit/pulse/utils.py @@ -43,33 +43,31 @@ def format_meas_map(meas_map: List[List[int]]) -> Dict[int, List[int]]: @functools.lru_cache(maxsize=None) def format_parameter_value( operand: ParameterExpression, + decimal: int = 10, ) -> Union[ParameterExpression, complex]: """Convert ParameterExpression into the most suitable data type. Args: operand: Operand value in arbitrary data type including ParameterExpression. + decimal: Number of digit to round returned value. Returns: Value casted to non-parameter data type, when possible. """ - # to evaluate parameter expression object, sympy srepr function is used. - # this function converts the parameter object into string with tiny round error. - # therefore evaluated value is not completely equal to the assigned value. - # however this error can be ignored in practice though we need to be careful for unittests. - # i.e. "pi=3.141592653589793" will be evaluated as "3.14159265358979" - # no DAC that recognizes the resolution of 1e-15 but they are AlmostEqual in tests. - from sympy import srepr - - math_expr = srepr(operand).replace("*I", "j") try: - # value is assigned - evaluated = complex(math_expr) - if not np.iscomplex(evaluated): + # value is assigned. + # note that ParameterExpression directly supports __complex__ via sympy or symengine + evaluated = complex(operand) + # remove truncation error + evaluated = np.round(evaluated, decimals=decimal) + # typecast into most likely data type + if np.isreal(evaluated): evaluated = float(evaluated.real) if evaluated.is_integer(): evaluated = int(evaluated) + return evaluated - except ValueError: + except TypeError: # value is not assigned pass diff --git a/test/python/pulse/test_parameter_manager.py b/test/python/pulse/test_parameter_manager.py index 5ccffe391fd9..c5d6c75219db 100644 --- a/test/python/pulse/test_parameter_manager.py +++ b/test/python/pulse/test_parameter_manager.py @@ -16,6 +16,8 @@ from copy import deepcopy +import numpy as np + from qiskit import pulse from qiskit.circuit import Parameter from qiskit.pulse.parameter_manager import ParameterGetter, ParameterSetter @@ -279,6 +281,60 @@ def test_nested_assignment_partial_bind(self): def test_complex_valued_parameter(self): """Test complex valued parameter can be casted to a complex value.""" 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) + + 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.""" + 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) + + 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.""" + amp = Parameter("amp") + + mag = Parameter("A") + phi = Parameter("phi") + + test_obj = pulse.Constant(duration=160, amp=amp) + + # generate parameter expression + value_dict = {amp: mag * np.exp(1j * phi)} + visitor = ParameterSetter(param_map=value_dict) + assigned = visitor.visit(test_obj) + + # generate complex value + value_dict = {mag: 0.1, phi: 0.5} + visitor = ParameterSetter(param_map=value_dict) + 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) + + self.assertEqual(assigned, ref_obj) + + def test_invalid_pulse_amplitude(self): + """Test that invalid parameters are still checked upon assignment.""" + amp = Parameter("amp") test_sched = pulse.ScheduleBlock() test_sched.append( diff --git a/test/python/pulse/test_parameters.py b/test/python/pulse/test_parameters.py index 894e0c17503b..41a8378cdbd1 100644 --- a/test/python/pulse/test_parameters.py +++ b/test/python/pulse/test_parameters.py @@ -12,14 +12,12 @@ """Test cases for parameters used in Schedules.""" import unittest -import cmath from copy import deepcopy import numpy as np from qiskit import pulse, assemble from qiskit.circuit import Parameter -from qiskit.circuit.parameterexpression import HAS_SYMENGINE from qiskit.pulse import PulseError from qiskit.pulse.channels import DriveChannel, AcquireChannel, MemorySlot from qiskit.pulse.transforms import inline_subroutines @@ -395,18 +393,7 @@ def test_assign_parameter_to_subroutine_parameter(self): self.assertEqual(len(main_prog.parameters), 2) target = deepcopy(main_prog).assign_parameters({param_sub1: 0.1, param_sub2: 0.5}) result = inline_subroutines(target) - if not HAS_SYMENGINE: - self.assertEqual(result, reference) - else: - # Because of simplification differences between sympy and symengine when - # symengine is used we get 0.1*exp(0.5*I) instead of the evaluated - # 0.0877582562 + 0.0479425539*I resulting in a failure. When - # symengine is installed manually build the amplitude as a complex to - # avoid this. - reference = pulse.Schedule() - waveform = pulse.library.Constant(duration=100, amp=0.1 * cmath.exp(0.5j)) - reference += pulse.Play(waveform, DriveChannel(0)) - self.assertEqual(result, reference) + self.assertEqual(result, reference) class TestParameterDuration(QiskitTestCase): From b2b52f5707d3202320c6c91f0013b4f981ae14cc Mon Sep 17 00:00:00 2001 From: knzwnao Date: Mon, 15 Nov 2021 18:24:00 +0900 Subject: [PATCH 2/2] reno --- ...-parameter-formatter-c9ff103f1a7181e0.yaml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 releasenotes/notes/fix-pulse-parameter-formatter-c9ff103f1a7181e0.yaml diff --git a/releasenotes/notes/fix-pulse-parameter-formatter-c9ff103f1a7181e0.yaml b/releasenotes/notes/fix-pulse-parameter-formatter-c9ff103f1a7181e0.yaml new file mode 100644 index 000000000000..644273e32e02 --- /dev/null +++ b/releasenotes/notes/fix-pulse-parameter-formatter-c9ff103f1a7181e0.yaml @@ -0,0 +1,20 @@ +--- +fixes: + - | + Complex valued pulse parameter assignment with symengine has been fixed. For example, + + .. code-block:: python + + from qiskit import circuit, pulse + import numpy as np + + amp = circuit.Parameter("amp") + phase = circuit.Parameter("phase") + + with pulse.build() as sched: + pulse.play(pulse.Gaussian(160, amp * np.exp(1j * phase), 40), pulse.DriveChannel(0)) + sched.assign_parameters({amp: 0.1, phase: 1.57}, inplace=True) + + The assigned amplitude has been shown as ``ParameterExpression(0.1*exp(1.57*I))`` + after symengine is introduced in #6270. This is now correctly + evaluated and shown as 7.96327e-05+0.0999999683j.