Improve parameter-binding performance of large instructions (#10284)

* Improve parameter-binding performance of large instructions

Previously, the parameter-assignment methods of `QuantumCircuit` had
poor performance when an instruction had a complex definition that
involved many of the parameters being bound.  The strategy of binding
each parameter separately led to each definition being copied and
rebound multiple times, with each rebinding being recursive all the way
down.

This commit makes the definition rebinding happen only once per
instruction, and updates the data model used to make it a complete
recursion through `QuantumCircuit.assign_parameters`.  This has the side
effect of fixing an issue where internal global phases would not be
updated.

The algorithmic change that enables this (just rebind the definition at
the end) is rather simpler than the length of this patch suggests.  This
is just because the previous structure of separating out a single
`_assign_parameter` method made it harder to restructure the logic
without introducing unpleasant stateful coupling between the driver and
helper methods.  Instead, I inlined most of the helper functions into
the driver body, so we can treat some components of the binding in a
per-parameter way and some in a per-operation way, in whatever way is
better.

* Fix lint

* Reduce overhead from input normalisation

This reduces several costs in the input normalisation, and makes the
calculation of some properties lazy for the sequence-like inputs; for
many close-to-hardware circuits such as those used in PEC, the sequence
form of parameter input is more natural, and almost no instructions will
have internally parametrised definitions, nor will there be a
parametrised global phase or calibrations.  In these cases, we can avoid
overhead from eagerly normalising the input into the forms that's easier
for these less-common assignment operations to use.

As a side-effect of the abstraction, we can also avoid making several
dictionary copies, and just use the mapping abstraction to filter the
dictionary during iteration on the fly.

This also takes the opportunity to improve the performance of sorting
large vectors of parameters.  In practice, I don't think this had a huge
impact on performance, but in principle it's rather more efficient now
and results in many fewer Python function calls during a sort.

* Address Ruff's generator concern

* Add comment on new keyword arguments

qiskit/circuit/_utils.py

@@ -16,6 +16,19 @@ This module contains utility functions for circuits.
 import numpy
 from qiskit.exceptions import QiskitError
 from qiskit.circuit.exceptions import CircuitError
+from .parametervector import ParameterVectorElement
+
+
+def sort_parameters(parameters):
+    """Sort an iterable of :class:`.Parameter` instances into a canonical order, respecting the
+    ordering relationships between elements of :class:`.ParameterVector`\\ s."""
+
+    def key(parameter):
+        if isinstance(parameter, ParameterVectorElement):
+            return (parameter.vector.name, parameter.index)
+        return (parameter.name,)
+
+    return sorted(parameters, key=key)
 
 
 def _compute_control_matrix(base_mat, num_ctrl_qubits, ctrl_state=None):

qiskit/circuit/equivalence.py

@@ -20,6 +20,7 @@ import rustworkx as rx
 
 from qiskit.exceptions import InvalidFileError
 from .exceptions import CircuitError
+from .parameter import Parameter
 from .parameterexpression import ParameterExpression
 
 Key = namedtuple("Key", ["name", "num_qubits"])
@@ -284,7 +285,7 @@ def _raise_if_shape_mismatch(gate, circuit):
 
 def _rebind_equiv(equiv, query_params):
     equiv_params, equiv_circuit = equiv
-    param_map = dict(zip(equiv_params, query_params))
-    equiv = equiv_circuit.assign_parameters(param_map, inplace=False)
+    param_map = {x: y for x, y in zip(equiv_params, query_params) if isinstance(x, Parameter)}
+    equiv = equiv_circuit.assign_parameters(param_map, inplace=False, flat_input=True)
 
     return equiv

qiskit/circuit/library/n_local/n_local.py

@@ -779,6 +779,7 @@ class NLocal(BlueprintCircuit):
         parameters: Mapping[Parameter, ParameterExpression | float]
         | Sequence[ParameterExpression | float],
         inplace: bool = False,
+        **kwargs,
     ) -> QuantumCircuit | None:
         """Assign parameters to the n-local circuit.
 
@@ -800,7 +801,7 @@ class NLocal(BlueprintCircuit):
         if not self._is_built:
             self._build()
 
-        return super().assign_parameters(parameters, inplace=inplace)
+        return super().assign_parameters(parameters, inplace=inplace, **kwargs)
 
     def _parameterize_block(
         self, block, param_iter=None, rep_num=None, block_num=None, indices=None, params=None

qiskit/circuit/quantumcircuit.py

@@ -18,7 +18,6 @@ from __future__ import annotations
 import collections.abc
 import copy
 import itertools
-import functools
 import multiprocessing as mp
 import string
 import re
@@ -49,12 +48,13 @@ from qiskit.circuit.parameter import Parameter
 from qiskit.circuit.exceptions import CircuitError
 from qiskit.utils import optionals as _optionals
 from . import _classical_resource_map
+from ._utils import sort_parameters
 from .classical import expr
 from .parameterexpression import ParameterExpression, ParameterValueType
 from .quantumregister import QuantumRegister, Qubit, AncillaRegister, AncillaQubit
 from .classicalregister import ClassicalRegister, Clbit
 from .parametertable import ParameterReferences, ParameterTable, ParameterView
-from .parametervector import ParameterVector, ParameterVectorElement
+from .parametervector import ParameterVector
 from .instructionset import InstructionSet
 from .operation import Operation
 from .register import Register
@@ -2614,9 +2614,7 @@ class QuantumCircuit:
         """
         # parameters from gates
         if self._parameters is None:
-            unsorted = self._unsorted_parameters()
-            self._parameters = sorted(unsorted, key=functools.cmp_to_key(_compare_parameters))
-
+            self._parameters = sort_parameters(self._unsorted_parameters())
         # return as parameter view, which implements the set and list interface
         return ParameterView(self._parameters)
 
@@ -2638,6 +2636,9 @@ class QuantumCircuit:
         self,
         parameters: Union[Mapping[Parameter, ParameterValueType], Sequence[ParameterValueType]],
         inplace: Literal[False] = ...,
+        *,
+        flat_input: bool = ...,
+        strict: bool = ...,
     ) -> "QuantumCircuit":
         ...
 
@@ -2646,13 +2647,19 @@ class QuantumCircuit:
         self,
         parameters: Union[Mapping[Parameter, ParameterValueType], Sequence[ParameterValueType]],
         inplace: Literal[True] = ...,
+        *,
+        flat_input: bool = ...,
+        strict: bool = ...,
     ) -> None:
         ...
 
-    def assign_parameters(
+    def assign_parameters(  # pylint: disable=missing-raises-doc
         self,
         parameters: Union[Mapping[Parameter, ParameterValueType], Sequence[ParameterValueType]],
         inplace: bool = False,
+        *,
+        flat_input: bool = False,
+        strict: bool = True,
     ) -> Optional["QuantumCircuit"]:
         """Assign parameters to new parameters or values.
 
@@ -2670,6 +2677,13 @@ class QuantumCircuit:
             parameters: Either a dictionary or iterable specifying the new parameter values.
             inplace: If False, a copy of the circuit with the bound parameters is returned.
                 If True the circuit instance itself is modified.
+            flat_input: If ``True`` and ``parameters`` is a mapping type, it is assumed to be
+                exactly a mapping of ``{parameter: value}``.  By default (``False``), the mapping
+                may also contain :class:`.ParameterVector` keys that point to a corresponding
+                sequence of values, and these will be unrolled during the mapping.
+            strict: If ``False``, any parameters given in the mapping that are not used in the
+                circuit will be ignored.  If ``True`` (the default), an error will be raised
+                indicating a logic error.
 
         Raises:
             CircuitError: If parameters is a dict and contains parameters not present in the
@@ -2678,7 +2692,7 @@ class QuantumCircuit:
                 parameters in the circuit.
 
         Returns:
-            A copy of the circuit with bound parameters, if ``inplace`` is False, otherwise None.
+            A copy of the circuit with bound parameters if ``inplace`` is False, otherwise None.
 
         Examples:
 
@@ -2715,47 +2729,154 @@ class QuantumCircuit:
                circuit.draw('mpl')
 
         """
-        # replace in self or in a copy depending on the value of in_place
         if inplace:
-            bound_circuit = self
+            target = self
         else:
-            bound_circuit = self.copy()
-            self._increment_instances()
-            bound_circuit._name_update()
+            target = self.copy()
+            target._increment_instances()
+            target._name_update()
 
+        # Normalise the inputs into simple abstract interfaces, so we've dispatched the "iteration"
+        # logic in one place at the start of the function.  This lets us do things like calculate
+        # and cache expensive properties for (e.g.) the sequence format only if they're used; for
+        # many large, close-to-hardware circuits, we won't need the extra handling for
+        # `global_phase` or recursive definition binding.
+        #
+        # During normalisation, be sure to reference 'parameters' and related things from 'self' not
+        # 'target' so we can take advantage of any caching we might be doing.
         if isinstance(parameters, dict):
-            # unroll the parameter dictionary (needed if e.g. it contains a ParameterVector)
-            unrolled_param_dict = self._unroll_param_dict(parameters)
-            unsorted_parameters = self._unsorted_parameters()
-
-            # check that all param_dict items are in the _parameter_table for this circuit
-            params_not_in_circuit = [
-                param_key
-                for param_key in unrolled_param_dict
-                if param_key not in unsorted_parameters
-            ]
-            if len(params_not_in_circuit) > 0:
+            raw_mapping = parameters if flat_input else self._unroll_param_dict(parameters)
+            our_parameters = self._unsorted_parameters()
+            if strict and (extras := raw_mapping.keys() - our_parameters):
                 raise CircuitError(
-                    "Cannot bind parameters ({}) not present in the circuit.".format(
-                        ", ".join(map(str, params_not_in_circuit))
-                    )
+                    f"Cannot bind parameters ({', '.join(str(x) for x in extras)}) not present in"
+                    " the circuit."
                 )
-
-            # replace the parameters with a new Parameter ("substitute") or numeric value ("bind")
-            for parameter, value in unrolled_param_dict.items():
-                bound_circuit._assign_parameter(parameter, value)
+            parameter_binds = _ParameterBindsDict(raw_mapping, our_parameters)
         else:
-            if len(parameters) != self.num_parameters:
+            our_parameters = self.parameters
+            if len(parameters) != len(our_parameters):
                 raise ValueError(
                     "Mismatching number of values and parameters. For partial binding "
                     "please pass a dictionary of {parameter: value} pairs."
                 )
-            # use a copy of the parameters, to ensure we don't change the contents of
-            # self.parameters while iterating over them
-            fixed_parameters_copy = self.parameters.copy()
-            for i, value in enumerate(parameters):
-                bound_circuit._assign_parameter(fixed_parameters_copy[i], value)
-        return None if inplace else bound_circuit
+            parameter_binds = _ParameterBindsSequence(our_parameters, parameters)
+
+        # Clear out the parameter table for the relevant entries, since we'll be binding those.
+        # Any new references to parameters are reinserted as part of the bind.
+        target._parameters = None
+        # This is deliberately eager, because we want the side effect of clearing the table.
+        all_references = [
+            (parameter, value, target._parameter_table.pop(parameter, ()))
+            for parameter, value in parameter_binds.items()
+        ]
+        seen_operations = {}
+        # The meat of the actual binding for regular operations.
+        for to_bind, bound_value, references in all_references:
+            update_parameters = (
+                tuple(bound_value.parameters)
+                if isinstance(bound_value, ParameterExpression)
+                else ()
+            )
+            for operation, index in references:
+                seen_operations[id(operation)] = operation
+                assignee = operation.params[index]
+                if isinstance(assignee, ParameterExpression):
+                    new_parameter = assignee.assign(to_bind, bound_value)
+                    for parameter in update_parameters:
+                        if parameter not in target._parameter_table:
+                            target._parameter_table[parameter] = ParameterReferences(())
+                        target._parameter_table[parameter].add((operation, index))
+                    if not new_parameter.parameters:
+                        if new_parameter.is_real():
+                            new_parameter = (
+                                int(new_parameter)
+                                if new_parameter._symbol_expr.is_integer
+                                else float(new_parameter)
+                            )
+                        else:
+                            new_parameter = complex(new_parameter)
+                        new_parameter = operation.validate_parameter(new_parameter)
+                elif isinstance(assignee, QuantumCircuit):
+                    new_parameter = assignee.assign_parameters(
+                        {to_bind: bound_value}, inplace=False, flat_input=True
+                    )
+                else:
+                    raise RuntimeError(  # pragma: no cover
+                        f"Saw an unknown type during symbolic binding: {assignee}."
+                        " This may indicate an internal logic error in symbol tracking."
+                    )
+                operation.params[index] = new_parameter
+
+        # After we've been through everything at the top level, make a single visit to each
+        # operation we've seen, rebinding its definition if necessary.
+        for operation in seen_operations.values():
+            if (
+                definition := getattr(operation, "_definition", None)
+            ) is not None and definition.num_parameters:
+                definition.assign_parameters(
+                    parameter_binds.mapping, inplace=True, flat_input=True, strict=False
+                )
+
+        if isinstance(target.global_phase, ParameterExpression):
+            new_phase = target.global_phase
+            for parameter in new_phase.parameters & parameter_binds.mapping.keys():
+                new_phase = new_phase.assign(parameter, parameter_binds.mapping[parameter])
+            target.global_phase = new_phase
+
+        # Finally, assign the parameters inside any of the calibrations.  We don't track these in
+        # the `ParameterTable`, so we manually reconstruct things.
+        def map_calibration(qubits, parameters, schedule):
+            modified = False
+            new_parameters = list(parameters)
+            for i, parameter in enumerate(new_parameters):
+                if not isinstance(parameter, ParameterExpression):
+                    continue
+                if not (contained := parameter.parameters & parameter_binds.mapping.keys()):
+                    continue
+                for to_bind in contained:
+                    parameter = parameter.assign(to_bind, parameter_binds.mapping[to_bind])
+                if not parameter.parameters:
+                    parameter = (
+                        int(parameter) if parameter._symbol_expr.is_integer else float(parameter)
+                    )
+                new_parameters[i] = parameter
+                modified = True
+            if modified:
+                schedule.assign_parameters(parameter_binds.mapping)
+            return (qubits, tuple(new_parameters)), schedule
+
+        target._calibrations = defaultdict(
+            dict,
+            (
+                (
+                    gate,
+                    dict(
+                        map_calibration(qubits, parameters, schedule)
+                        for (qubits, parameters), schedule in calibrations.items()
+                    ),
+                )
+                for gate, calibrations in target._calibrations.items()
+            ),
+        )
+        return None if inplace else target
+
+    @staticmethod
+    def _unroll_param_dict(
+        parameter_binds: Mapping[Parameter, ParameterValueType]
+    ) -> Mapping[Parameter, ParameterValueType]:
+        out = {}
+        for parameter, value in parameter_binds.items():
+            if isinstance(parameter, ParameterVector):
+                if len(parameter) != len(value):
+                    raise CircuitError(
+                        f"Parameter vector '{parameter.name}' has length {len(parameter)},"
+                        f" but was assigned to {len(value)} values."
+                    )
+                out.update(zip(parameter, value))
+            else:
+                out[parameter] = value
+        return out
 
     def bind_parameters(
         self, values: Union[Mapping[Parameter, float], Sequence[float]]
@@ -2791,136 +2912,6 @@ class QuantumCircuit:
                 )
             return self.assign_parameters(values)
 
-    def _unroll_param_dict(
-        self, value_dict: Mapping[Parameter, ParameterValueType]
-    ) -> dict[Parameter, ParameterValueType]:
-        unrolled_value_dict: dict[Parameter, ParameterValueType] = {}
-        for (param, value) in value_dict.items():
-            if isinstance(param, ParameterVector):
-                if not len(param) == len(value):
-                    raise CircuitError(
-                        "ParameterVector {} has length {}, which "
-                        "differs from value list {} of "
-                        "len {}".format(param, len(param), value, len(value))
-                    )
-                unrolled_value_dict.update(zip(param, value))
-            # pass anything else except number through. error checking is done in assign_parameter
-            elif isinstance(param, (ParameterExpression, str)) or param is None:
-                unrolled_value_dict[param] = value
-        return unrolled_value_dict
-
-    def _assign_parameter(self, parameter: Parameter, value: ParameterValueType) -> None:
-        """Update this circuit where instances of ``parameter`` are replaced by ``value``, which
-        can be either a numeric value or a new parameter expression.
-
-        Args:
-            parameter (ParameterExpression): Parameter to be bound
-            value (Union(ParameterExpression, float, int)): A numeric or parametric expression to
-                replace instances of ``parameter``.
-
-        Raises:
-            RuntimeError: if some internal logic error has caused the circuit instruction sequence
-                and the parameter table to become out of sync, and the table now contains a
-                reference to a value that cannot be assigned.
-        """
-        # parameter might be in global phase only
-        if parameter in self._parameter_table:
-            for instr, param_index in self._parameter_table[parameter]:
-                assignee = instr.params[param_index]
-                # Normal ParameterExpression.
-                if isinstance(assignee, ParameterExpression):
-                    new_param = assignee.assign(parameter, value)
-                    # if fully bound, validate
-                    if len(new_param.parameters) == 0:
-                        if new_param._symbol_expr.is_integer and new_param.is_real():
-                            val = int(new_param)
-                        elif new_param.is_real():
-                            val = float(new_param)
-                        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
-
-                    self._rebind_definition(instr, parameter, value)
-                # Scoped block of a larger instruction.
-                elif isinstance(assignee, QuantumCircuit):
-                    # It's possible that someone may re-use a loop body, so we need to mutate the
-                    # parameter vector with a new circuit, rather than mutating the body.
-                    instr.params[param_index] = assignee.assign_parameters({parameter: value})
-                else:
-                    raise RuntimeError(  # pragma: no cover
-                        "The ParameterTable or data of this QuantumCircuit have become out-of-sync."
-                        f"\nParameterTable: {self._parameter_table}"
-                        f"\nData: {self.data}"
-                    )
-
-            if isinstance(value, ParameterExpression):
-                entry = self._parameter_table.pop(parameter)
-                for new_parameter in value.parameters:
-                    if new_parameter in self._parameter_table:
-                        self._parameter_table[new_parameter] |= entry
-                    else:
-                        self._parameter_table[new_parameter] = entry
-            else:
-                del self._parameter_table[parameter]  # clear evaluated expressions
-
-        if (
-            isinstance(self.global_phase, ParameterExpression)
-            and parameter in self.global_phase.parameters
-        ):
-            self.global_phase = self.global_phase.assign(parameter, value)
-
-        # clear parameter cache
-        self._parameters = None
-        self._assign_calibration_parameters(parameter, value)
-
-    def _assign_calibration_parameters(
-        self, parameter: Parameter, value: ParameterValueType
-    ) -> None:
-        """Update parameterized pulse gate calibrations, if there are any which contain
-        ``parameter``. This updates the calibration mapping as well as the gate definition
-        ``Schedule``s, which also may contain ``parameter``.
-        """
-        new_param: ParameterValueType
-        for cals in self.calibrations.values():
-            for (qubit, cal_params), schedule in copy.copy(cals).items():
-                if any(
-                    isinstance(p, ParameterExpression) and parameter in p.parameters
-                    for p in cal_params
-                ):
-                    del cals[(qubit, cal_params)]
-                    new_cal_params = []
-                    for p in cal_params:
-                        if isinstance(p, ParameterExpression) and parameter in p.parameters:
-                            new_param = p.assign(parameter, value)
-                            if not new_param.parameters:
-                                if new_param._symbol_expr.is_integer:
-                                    new_param = int(new_param)
-                                else:
-                                    new_param = float(new_param)
-                            new_cal_params.append(new_param)
-                        else:
-                            new_cal_params.append(p)
-                    schedule.assign_parameters({parameter: value})
-                    cals[(qubit, tuple(new_cal_params))] = schedule
-
-    def _rebind_definition(
-        self, instruction: Instruction, parameter: Parameter, value: ParameterValueType
-    ) -> None:
-        if instruction._definition:
-            for inner in instruction._definition:
-                for idx, param in enumerate(inner.operation.params):
-                    if isinstance(param, ParameterExpression) and parameter in param.parameters:
-                        if isinstance(value, ParameterExpression):
-                            inner.operation.params[idx] = param.subs({parameter: value})
-                        else:
-                            inner.operation.params[idx] = param.bind({parameter: value})
-                        self._rebind_definition(inner.operation, parameter, value)
-
     def barrier(self, *qargs: QubitSpecifier, label=None) -> InstructionSet:
         """Apply :class:`~.library.Barrier`. If ``qargs`` is empty, applies to all qubits
         in the circuit.
@@ -4970,22 +4961,40 @@ class QuantumCircuit:
         return 0  # If there are no instructions over bits
 
 
-def _standard_compare(value1, value2):
-    if value1 < value2:
-        return -1
-    if value1 > value2:
-        return 1
-    return 0
+class _ParameterBindsDict:
+    __slots__ = ("mapping", "allowed_keys")
+
+    def __init__(self, mapping, allowed_keys):
+        self.mapping = mapping
+        self.allowed_keys = allowed_keys
+
+    def items(self):
+        """Iterator through all the keys in the mapping that we care about.  Wrapping the main
+        mapping allows us to avoid reconstructing a new 'dict', but just use the given 'mapping'
+        without any copy / reconstruction."""
+        for parameter, value in self.mapping.items():
+            if parameter in self.allowed_keys:
+                yield parameter, value
 
 
-def _compare_parameters(param1: Parameter, param2: Parameter) -> int:
-    if isinstance(param1, ParameterVectorElement) and isinstance(param2, ParameterVectorElement):
-        # if they belong to a vector with the same name, sort by index
-        if param1.vector.name == param2.vector.name:
-            return _standard_compare(param1.index, param2.index)
+class _ParameterBindsSequence:
+    __slots__ = ("parameters", "values", "mapping_cache")
 
-    # else sort by name
-    return _standard_compare(param1.name, param2.name)
+    def __init__(self, parameters, values):
+        self.parameters = parameters
+        self.values = values
+        self.mapping_cache = None
+
+    def items(self):
+        """Iterator through all the keys in the mapping that we care about."""
+        return zip(self.parameters, self.values)
+
+    @property
+    def mapping(self):
+        """Cached version of a mapping.  This is only generated on demand."""
+        if self.mapping_cache is None:
+            self.mapping_cache = dict(zip(self.parameters, self.values))
+        return self.mapping_cache
 
 
 # Used by the OQ2 exporter.  Just needs to have enough parameters to support the largest standard

qiskit/opflow/gradients/gradient.py

@@ -13,11 +13,10 @@
 """The base interface for Opflow's gradient."""
 
 from typing import Union, List, Optional
-import functools
 import numpy as np
 
-from qiskit.circuit.quantumcircuit import _compare_parameters
 from qiskit.circuit import ParameterExpression, ParameterVector
+from qiskit.circuit._utils import sort_parameters
 from qiskit.utils import optionals as _optionals
 from qiskit.utils.deprecation import deprecate_func
 from .circuit_gradients.circuit_gradient import CircuitGradient
@@ -67,7 +66,7 @@ class Gradient(GradientBase):
         if len(operator.parameters) == 0:
             raise ValueError("The operator we are taking the gradient of is not parameterized!")
         if params is None:
-            params = sorted(operator.parameters, key=functools.cmp_to_key(_compare_parameters))
+            params = sort_parameters(operator.parameters)
         if isinstance(params, (ParameterVector, list)):
             param_grads = [self.convert(operator, param) for param in params]
             absent_params = [

qiskit/opflow/gradients/hessian.py

@@ -13,11 +13,10 @@
 """The module to compute Hessians."""
 
 from typing import Union, List, Tuple, Optional
-import functools
 import numpy as np
 
-from qiskit.circuit.quantumcircuit import _compare_parameters
 from qiskit.circuit import ParameterVector, ParameterExpression
+from qiskit.circuit._utils import sort_parameters
 from qiskit.utils import optionals as _optionals
 from qiskit.utils.deprecation import deprecate_func
 from ..operator_globals import Zero, One
@@ -115,7 +114,7 @@ class Hessian(HessianBase):
         if len(operator.parameters) == 0:
             raise ValueError("The operator we are taking the gradient of is not parameterized!")
         if params is None:
-            params = sorted(operator.parameters, key=functools.cmp_to_key(_compare_parameters))
+            params = sort_parameters(operator.parameters)
         # if input is a tuple instead of a list, wrap it into a list
         if isinstance(params, (ParameterVector, list)):
             # Case: a list of parameters were given, compute the Hessian for all param pairs

qiskit/opflow/gradients/natural_gradient.py

@@ -14,11 +14,10 @@
 
 from collections.abc import Iterable
 from typing import List, Tuple, Callable, Optional, Union
-import functools
 import numpy as np
 
-from qiskit.circuit.quantumcircuit import _compare_parameters
 from qiskit.circuit import ParameterVector, ParameterExpression
+from qiskit.circuit._utils import sort_parameters
 from qiskit.utils import optionals as _optionals
 from qiskit.utils.deprecation import deprecate_func
 from ..operator_base import OperatorBase
@@ -122,7 +121,7 @@ class NaturalGradient(GradientBase):
         if len(operator.parameters) == 0:
             raise ValueError("The operator we are taking the gradient of is not parameterized!")
         if params is None:
-            params = sorted(operator.parameters, key=functools.cmp_to_key(_compare_parameters))
+            params = sort_parameters(operator.parameters)
         if not isinstance(params, Iterable):
             params = [params]
         # Instantiate the gradient

qiskit/opflow/gradients/qfi.py

@@ -13,10 +13,9 @@
 """The module for Quantum the Fisher Information."""
 
 from typing import List, Union, Optional
-import functools
 
-from qiskit.circuit.quantumcircuit import _compare_parameters
 from qiskit.circuit import ParameterExpression, ParameterVector
+from qiskit.circuit._utils import sort_parameters
 from qiskit.utils.deprecation import deprecate_func
 from ..list_ops.list_op import ListOp
 from ..expectations.pauli_expectation import PauliExpectation
@@ -71,5 +70,5 @@ class QFI(QFIBase):
         cleaned_op = self._factor_coeffs_out_of_composed_op(expec_op)
 
         if params is None:
-            params = sorted(operator.parameters, key=functools.cmp_to_key(_compare_parameters))
+            params = sort_parameters(operator.parameters)
         return self.qfi_method.convert(cleaned_op, params)

releasenotes/notes/faster-parameter-rebind-3c799e74456469d9.yaml

@@ -0,0 +1,20 @@
+---
+features:
+  - |
+    The performance of :meth:`.QuantumCircuit.assign_parameters` and :meth:`~.QuantumCircuit.bind_parameters`
+    has significantly increased for large circuits with structures typical of applications uses.
+    This includes most circuits based on the :class:`.NLocal` structure, such as
+    :class:`.EfficientSU2`.  See `#10282 <https://github.com/Qiskit/qiskit-terra/issues/10282>`__ for more
+    detail.
+  - |
+    The method :meth:`.QuantumCircuit.assign_parameters` has gained two new keywords arguments: ``flat_input``
+    and ``strict``.  These are advanced options that can be used to speed up the method when passing the
+    parameter bindings as a dictionary; ``flat_input=True`` is a guarantee that the dictionary keys contain
+    only :class:`.Parameter` instances (not :class:`.ParameterVector`\ s), and ``strict=False`` allows the
+    dictionary to contain parameters that are not present in the circuit.  Using these two options can
+    reduce the overhead of input normalisation in this function.
+fixes:
+  - |
+    A parametrised circuit that contains a custom gate whose definition has a parametrised global phase
+    can now successfully bind the parameter in the inner global phase.
+    See `#10283 <https://github.com/Qiskit/qiskit-terra/issues/10283>`__ for more detail.

test/python/circuit/test_parameters.py

@@ -193,6 +193,18 @@ class TestParameters(QiskitTestCase):
         c = a.bind({a: 1, b: 1}, allow_unknown_parameters=True)
         self.assertEqual(c, a.bind({a: 1}))
 
+    @data(QuantumCircuit.assign_parameters, QuantumCircuit.bind_parameters)
+    def test_bind_parameters_custom_definition_global_phase(self, assigner):
+        """Test that a custom gate with a parametrised `global_phase` is assigned correctly."""
+        x = Parameter("x")
+        custom = QuantumCircuit(1, global_phase=x).to_gate()
+        base = QuantumCircuit(1)
+        base.append(custom, [0], [])
+
+        test = Operator(assigner(base, {x: math.pi}))
+        expected = Operator(numpy.array([[-1, 0], [0, -1]]))
+        self.assertEqual(test, expected)
+
     def test_bind_half_single_precision(self):
         """Test binding with 16bit and 32bit floats."""
         phase = Parameter("phase")

test/python/qasm3/test_export.py

@@ -432,7 +432,7 @@ class TestCircuitQASM3(QiskitTestCase):
             [
                 "OPENQASM 3;",
                 'include "stdgates.inc";',
-                "gate custom(p0, p1) _gate_q_0, _gate_q_1 {",
+                "gate custom(_gate_p_0, _gate_p_0) _gate_q_0, _gate_q_1 {",
                 "  rz(pi) _gate_q_0;",
                 "  rz(pi/4) _gate_q_1;",
                 "}",
@@ -1670,7 +1670,7 @@ class TestCircuitQASM3ExporterTemporaryCasesWithBadParameterisation(QiskitTestCa
             [
                 "OPENQASM 3;",
                 'include "stdgates.inc";',
-                f"gate custom_{custom_id}(p0, p1) _gate_q_0, _gate_q_1 {{",
+                f"gate custom_{custom_id}(_gate_p_0, _gate_p_1) _gate_q_0, _gate_q_1 {{",
                 "  rz(pi) _gate_q_0;",
                 "  rz(pi/4) _gate_q_1;",
                 "}",