Fix pulse parameter value formatter bug (#11972)

* Fix value formatter bug

* reno

* Comments from review

Co-authored-by: Will Shanks <wshaos@posteo.net>

---------

Co-authored-by: Will Shanks <wshaos@posteo.net>

qiskit/pulse/utils.py

@@ -52,17 +52,29 @@ def format_parameter_value(
     Returns:
         Value casted to non-parameter data type, when possible.
     """
-    try:
-        # 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)
+    if isinstance(operand, ParameterExpression):
+        try:
+            operand = operand.numeric()
+        except TypeError:
+            # Unassigned expression
+            return operand
+
+    # Return integer before calling the numpy round function.
+    # The input value is multiplied by 10**decimals, rounds to an integer
+    # and divided by 10**decimals. For a large enough integer,
+    # this operation may introduce a rounding error in the float operations
+    # and accidentally returns a float number.
+    if isinstance(operand, int):
+        return operand
+
+    # Remove truncation error and convert the result into Python builtin type.
+    # Value could originally contain a rounding error, e.g. 1.00000000001
+    # which may occur during the parameter expression evaluation.
+    evaluated = np.round(operand, decimals=decimal).item()
+
+    if isinstance(evaluated, complex):
+        if np.isclose(evaluated.imag, 0.0):
+            evaluated = evaluated.real
         else:
             warnings.warn(
                 "Assignment of complex values to ParameterExpression in Qiskit Pulse objects is "
@@ -76,13 +88,11 @@ def format_parameter_value(
                 "converted in a similar fashion to avoid the use of complex parameters.",
                 PendingDeprecationWarning,
             )
-
-        return evaluated
-    except TypeError:
-        # value is not assigned
-        pass
-
-    return operand
+            return evaluated
+    # Type cast integer-like float into Python builtin integer, after rounding.
+    if evaluated.is_integer():
+        return int(evaluated)
+    return evaluated
 
 
 def instruction_duration_validation(duration: int):

releasenotes/notes/fix-pulse-parameter-formatter-2ee3fb91efb2794c.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed a bug in :func:`qiskit.pulse.utils.format_parameter_value` function that
+    unintentionally converts large enough integer numbers into float values
+    or causes unexpected rounding.
+    See `qiskit/#11971 <https://github.com/Qiskit/qiskit/issues/11971>`__  for details.

test/python/pulse/test_parameter_manager.py

@@ -17,6 +17,7 @@
 from copy import deepcopy
 from unittest.mock import patch
 
+import ddt
 import numpy as np
 
 from qiskit import pulse
@@ -24,6 +25,7 @@ from qiskit.circuit import Parameter
 from qiskit.pulse.exceptions import PulseError, UnassignedDurationError
 from qiskit.pulse.parameter_manager import ParameterGetter, ParameterSetter
 from qiskit.pulse.transforms import AlignEquispaced, AlignLeft, inline_subroutines
+from qiskit.pulse.utils import format_parameter_value
 from test import QiskitTestCase  # pylint: disable=wrong-import-order
 
 
@@ -557,3 +559,80 @@ class TestScheduleTimeslots(QiskitTestCase):
         sched = pulse.Schedule()
         with self.assertRaises(UnassignedDurationError):
             sched.insert(0, test_play)
+
+
+@ddt.ddt
+class TestFormatParameter(QiskitTestCase):
+    """Test format_parameter_value function."""
+
+    def test_format_unassigned(self):
+        """Format unassigned parameter expression."""
+        p1 = Parameter("P1")
+        p2 = Parameter("P2")
+        expr = p1 + p2
+
+        self.assertEqual(format_parameter_value(expr), expr)
+
+    def test_partly_unassigned(self):
+        """Format partly assigned parameter expression."""
+        p1 = Parameter("P1")
+        p2 = Parameter("P2")
+        expr = (p1 + p2).assign(p1, 3.0)
+
+        self.assertEqual(format_parameter_value(expr), expr)
+
+    @ddt.data(1, 1.0, 1.00000000001, np.int64(1))
+    def test_integer(self, value):
+        """Format integer parameter expression."""
+        p1 = Parameter("P1")
+        expr = p1.assign(p1, value)
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, int)
+        self.assertEqual(out, 1)
+
+    @ddt.data(1.2, np.float64(1.2))
+    def test_float(self, value):
+        """Format float parameter expression."""
+        p1 = Parameter("P1")
+        expr = p1.assign(p1, value)
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, float)
+        self.assertEqual(out, 1.2)
+
+    @ddt.data(1.2 + 3.4j, np.complex128(1.2 + 3.4j))
+    def test_complex(self, value):
+        """Format float parameter expression."""
+        p1 = Parameter("P1")
+        expr = p1.assign(p1, value)
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, complex)
+        self.assertEqual(out, 1.2 + 3.4j)
+
+    def test_complex_rounding_error(self):
+        """Format float parameter expression."""
+        p1 = Parameter("P1")
+        expr = p1.assign(p1, 1.2 + 1j * 1e-20)
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, float)
+        self.assertEqual(out, 1.2)
+
+    def test_builtin_float(self):
+        """Format float parameter expression."""
+        expr = 1.23
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, float)
+        self.assertEqual(out, 1.23)
+
+    @ddt.data(15482812500000, 8465625000000, 4255312500000)
+    def test_edge_case(self, edge_case_val):
+        """Format integer parameter expression with
+        a particular integer number that causes rounding error at typecast."""
+
+        # Numbers to be tested here are chosen randomly.
+        # These numbers had caused mis-typecast into float before qiskit/#11972.
+
+        p1 = Parameter("P1")
+        expr = p1.assign(p1, edge_case_val)
+        out = format_parameter_value(expr)
+        self.assertIsInstance(out, int)
+        self.assertEqual(out, edge_case_val)