Assign values directly to fully bound parameters in quantum circuits (#…

…10183) (#10192)

* Assign circuit parameters as int/float to instructions

* Do not test that fully bound parameters are still ParameterExpressions

* Change int to float in qasm output

pi_check casts integers to floats but not integers inside
ParameterExpressions.

* Workaround symengine ComplexDouble not supporting float

* black

* Update releasenotes/notes/circuit-assign-parameter-to-concrete-value-7cad75c97183257f.yaml

Co-authored-by: Jake Lishman <[email protected]>

* Update releasenotes/notes/circuit-assign-parameter-to-concrete-value-7cad75c97183257f.yaml

Co-authored-by: Jake Lishman <[email protected]>

* Restore assigned parameter value type check to tests

* Add test to check type and value of simple circuit parameter assignment

* Add consistency check between assigned instruction data and calibrations dict keys

* Add regression test

* Add upgrade note

* Remove support for complex instruction parameter assignment

* Restore complex assignment

Complex assignment maybe not be supported but allowing it in the
parameter assignment step lets validate_parameters get the value and
raise an appropriate exception.

* black

* More specific assertion methods

* Use exact floating-point check

---------

Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
(cherry picked from commit 332bd9f)

Co-authored-by: Will Shanks <[email protected]>

qiskit/circuit/quantumcircuit.py

@@ -2829,7 +2829,17 @@ def _assign_parameter(self, parameter: Parameter, value: ParameterValueType) ->
                     new_param = assignee.assign(parameter, value)
                     # if fully bound, validate
                     if len(new_param.parameters) == 0:
-                        instr.params[param_index] = instr.validate_parameter(new_param)
+                        if new_param._symbol_expr.is_integer and new_param.is_real():
+                            val = int(new_param)
+                        elif new_param.is_real():
+                            # Workaround symengine not supporting float(<ComplexDouble>)
+                            val = complex(new_param).real
+                        else:
+                            # complex values may no longer be supported but we
+                            # defer raising an exception to validdate_parameter
+                            # below for now.
+                            val = complex(new_param)
+                        instr.params[param_index] = instr.validate_parameter(val)
                     else:
                         instr.params[param_index] = new_param
 
@@ -2886,7 +2896,11 @@ def _assign_calibration_parameters(
                         if isinstance(p, ParameterExpression) and parameter in p.parameters:
                             new_param = p.assign(parameter, value)
                             if not new_param.parameters:
-                                new_param = float(new_param)
+                                if new_param._symbol_expr.is_integer:
+                                    new_param = int(new_param)
+                                else:
+                                    # Workaround symengine not supporting float(<ComplexDouble>)
+                                    new_param = complex(new_param).real
                             new_cal_params.append(new_param)
                         else:
                             new_cal_params.append(p)

releasenotes/notes/circuit-assign-parameter-to-concrete-value-7cad75c97183257f.yaml

@@ -0,0 +1,29 @@
+---
+fixes:
+  - |
+    Changed the binding of numeric values with
+    :meth:`.QuantumCircuit.assign_parameters` to avoid a mismatch between the
+    values of circuit instruction parameters and corresponding parameter keys
+    in the circuit's calibration dictionary. Fixed `#9764
+    <https://github.com/Qiskit/qiskit-terra/issues/9764>`_ and `#10166
+    <https://github.com/Qiskit/qiskit-terra/issues/10166>`_. See also the
+    related upgrade note regarding :meth:`.QuantumCircuit.assign_parameters`.
+upgrade:
+  - |
+    Changed :meth:`.QuantumCircuit.assign_parameters` to bind
+    assigned integer and float values directly into the parameters of
+    :class:`~qiskit.circuit.Instruction` instances in the circuit rather than
+    binding the values wrapped within a
+    :class:`~qiskit.circuit.ParameterExpression`. This change should have
+    little user impact as ``float(QuantumCircuit.data[i].operation.params[j])``
+    still produces a ``float`` (and is the only way to access the value of a
+    :class:`~qiskit.circuit.ParameterExpression`). Also,
+    :meth:`~qiskit.circuit.Instruction` parameters could already be ``float``
+    as well as a :class:`~qiskit.circuit.ParameterExpression`, so code dealing
+    with instruction parameters should already handle both cases. The most
+    likely chance for user impact is in code that uses ``isinstance`` to check
+    for :class:`~qiskit.circuit.ParameterExpression` and behaves differently
+    depending on the result. Additionally, qpy serializes the numeric value in
+    a bound :class:`~qiskit.circuit.ParameterExpression` at a different
+    precision than a ``float`` (see also the related bug fix note about
+    :meth:`.QuantumCircuit.assign_parameters`).

test/python/circuit/test_circuit_load_from_qpy.py

@@ -19,7 +19,7 @@
 
 import numpy as np
 
-from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister
+from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister, pulse
 from qiskit.circuit import CASE_DEFAULT
 from qiskit.circuit.classicalregister import Clbit
 from qiskit.circuit.quantumregister import Qubit
@@ -274,6 +274,39 @@ def test_bound_parameter(self):
         self.assertEqual(qc, new_circ)
         self.assertDeprecatedBitProperties(qc, new_circ)
 
+    def test_bound_calibration_parameter(self):
+        """Test a circuit with a bound calibration parameter is correctly serialized.
+
+        In particular, this test ensures that parameters on a circuit
+        instruction are consistent with the circuit's calibrations dictionary
+        after serialization.
+        """
+        amp = Parameter("amp")
+
+        with pulse.builder.build() as sched:
+            pulse.builder.play(pulse.Constant(100, amp), pulse.DriveChannel(0))
+
+        gate = Gate("custom", 1, [amp])
+
+        qc = QuantumCircuit(1)
+        qc.append(gate, (0,))
+        qc.add_calibration(gate, (0,), sched)
+        qc.assign_parameters({amp: 1 / 3}, inplace=True)
+
+        qpy_file = io.BytesIO()
+        dump(qc, qpy_file)
+        qpy_file.seek(0)
+        new_circ = load(qpy_file)[0]
+        self.assertEqual(qc, new_circ)
+        instruction = new_circ.data[0]
+        cal_key = (
+            tuple(new_circ.find_bit(q).index for q in instruction.qubits),
+            tuple(instruction.operation.params),
+        )
+        # Make sure that looking for a calibration based on the instruction's
+        # parameters succeeds
+        self.assertIn(cal_key, new_circ.calibrations[gate.name])
+
     def test_parameter_expression(self):
         """Test a circuit with a parameter expression."""
         theta = Parameter("theta")

test/python/circuit/test_parameters.py

@@ -21,7 +21,7 @@
 from test import combine
 
 import numpy
-from ddt import data, ddt
+from ddt import data, ddt, named_data
 
 import qiskit
 import qiskit.circuit.library as circlib
@@ -346,6 +346,23 @@ def test_multiple_named_parameters(self):
         self.assertEqual(theta.name, "θ")
         self.assertEqual(qc.parameters, {theta, x})
 
+    @named_data(
+        ["int", 2, int],
+        ["float", 2.5, float],
+        ["float16", numpy.float16(2.5), float],
+        ["float32", numpy.float32(2.5), float],
+        ["float64", numpy.float64(2.5), float],
+    )
+    def test_circuit_assignment_to_numeric(self, value, type_):
+        """Test binding a numeric value to a circuit instruction"""
+        x = Parameter("x")
+        qc = QuantumCircuit(1)
+        qc.append(Instruction("inst", 1, 0, [x]), (0,))
+        qc.assign_parameters({x: value}, inplace=True)
+        bound = qc.data[0].operation.params[0]
+        self.assertIsInstance(bound, type_)
+        self.assertEqual(bound, value)
+
     def test_partial_binding(self):
         """Test that binding a subset of circuit parameters returns a new parameterized circuit."""
         theta = Parameter("θ")
@@ -401,10 +418,10 @@ def test_expression_partial_binding(self):
                 self.assertTrue(isinstance(pqc.data[0].operation.params[0], ParameterExpression))
                 self.assertEqual(str(pqc.data[0].operation.params[0]), "phi + 2")
 
-                fbqc = getattr(pqc, assign_fun)({phi: 1})
+                fbqc = getattr(pqc, assign_fun)({phi: 1.0})
 
                 self.assertEqual(fbqc.parameters, set())
-                self.assertTrue(isinstance(fbqc.data[0].operation.params[0], ParameterExpression))
+                self.assertIsInstance(fbqc.data[0].operation.params[0], float)
                 self.assertEqual(float(fbqc.data[0].operation.params[0]), 3)
 
     def test_two_parameter_expression_binding(self):
@@ -448,7 +465,7 @@ def test_expression_partial_binding_zero(self):
                 fbqc = getattr(pqc, assign_fun)({phi: 1})
 
                 self.assertEqual(fbqc.parameters, set())
-                self.assertTrue(isinstance(fbqc.data[0].operation.params[0], ParameterExpression))
+                self.assertIsInstance(fbqc.data[0].operation.params[0], int)
                 self.assertEqual(float(fbqc.data[0].operation.params[0]), 0)
 
     def test_raise_if_assigning_params_not_in_circuit(self):
@@ -505,8 +522,15 @@ def test_calibration_assignment(self):
         circ.add_calibration("rxt", [0], rxt_q0, [theta])
         circ = circ.assign_parameters({theta: 3.14})
 
-        self.assertTrue(((0,), (3.14,)) in circ.calibrations["rxt"])
-        sched = circ.calibrations["rxt"][((0,), (3.14,))]
+        instruction = circ.data[0]
+        cal_key = (
+            tuple(circ.find_bit(q).index for q in instruction.qubits),
+            tuple(instruction.operation.params),
+        )
+        self.assertEqual(cal_key, ((0,), (3.14,)))
+        # Make sure that key from instruction data matches the calibrations dictionary
+        self.assertIn(cal_key, circ.calibrations["rxt"])
+        sched = circ.calibrations["rxt"][cal_key]
         self.assertEqual(sched.instructions[0][1].pulse.amp, 0.2)
 
     def test_calibration_assignment_doesnt_mutate(self):
@@ -531,11 +555,11 @@ def test_calibration_assignment_doesnt_mutate(self):
         self.assertNotEqual(assigned_circ.calibrations, circ.calibrations)
 
     def test_calibration_assignment_w_expressions(self):
-        """That calibrations with multiple parameters and more expressions."""
+        """That calibrations with multiple parameters are assigned correctly"""
         theta = Parameter("theta")
         sigma = Parameter("sigma")
         circ = QuantumCircuit(3, 3)
-        circ.append(Gate("rxt", 1, [theta, sigma]), [0])
+        circ.append(Gate("rxt", 1, [theta / 2, sigma]), [0])
         circ.measure(0, 0)
 
         rxt_q0 = pulse.Schedule(
@@ -548,8 +572,15 @@ def test_calibration_assignment_w_expressions(self):
         circ.add_calibration("rxt", [0], rxt_q0, [theta / 2, sigma])
         circ = circ.assign_parameters({theta: 3.14, sigma: 4})
 
-        self.assertTrue(((0,), (3.14 / 2, 4)) in circ.calibrations["rxt"])
-        sched = circ.calibrations["rxt"][((0,), (3.14 / 2, 4))]
+        instruction = circ.data[0]
+        cal_key = (
+            tuple(circ.find_bit(q).index for q in instruction.qubits),
+            tuple(instruction.operation.params),
+        )
+        self.assertEqual(cal_key, ((0,), (3.14 / 2, 4)))
+        # Make sure that key from instruction data matches the calibrations dictionary
+        self.assertIn(cal_key, circ.calibrations["rxt"])
+        sched = circ.calibrations["rxt"][cal_key]
         self.assertEqual(sched.instructions[0][1].pulse.amp, 0.2)
         self.assertEqual(sched.instructions[0][1].pulse.sigma, 16)
 
@@ -789,7 +820,7 @@ def test_binding_parameterized_circuits_built_in_multiproc(self):
         for qc in results:
             circuit.compose(qc, inplace=True)
 
-        parameter_values = [{x: 1 for x in parameters}]
+        parameter_values = [{x: 1.0 for x in parameters}]
 
         qobj = assemble(
             circuit,
@@ -802,7 +833,7 @@ def test_binding_parameterized_circuits_built_in_multiproc(self):
         self.assertTrue(
             all(
                 len(inst.params) == 1
-                and isinstance(inst.params[0], ParameterExpression)
+                and isinstance(inst.params[0], float)
                 and float(inst.params[0]) == 1
                 for inst in qobj.experiments[0].instructions
             )

test/python/qasm3/test_export.py

@@ -469,7 +469,7 @@ def test_reused_custom_parameter(self):
                 "  rx(0.5) _gate_q_0;",
                 "}",
                 f"gate {circuit_name_1} _gate_q_0 {{",
-                "  rx(1) _gate_q_0;",
+                "  rx(1.0) _gate_q_0;",
                 "}",
                 "qubit[1] _all_qubits;",
                 "let q = _all_qubits[0:0];",