Fix performance regression in looped QuantumCircuit.assign_parameters

[jakelishman]

Summary

When calling assign_parameters on a heavily parametric circuit with the unusual access pattern of binding off a single parameter at a time, Qiskit 1.2 had a severe performance regression compared to Qiskit 1.1 stemming from gh-12794. The calls to unsorted_parameters on each iteration were creating a new set, which could be huge if the number of parameters in the circuit was large. In Qiskit 1.1 and before, that object was a direct view onto the underlying ParameterTable (assuming the input circuit did not have a parametric global phase), so was free to construct.

Details and comments

For example, using a circuit like

from qiskit.circuit import QuantumCircuit, Parameter

def make(num_qubits, num_reps):
    initials = [Parameter(str(i)) for i in range(num_qubits * num_reps)]
    qc = QuantumCircuit(num_qubits)
    ps = iter(initials)
    for _ in range(num_reps):
        for q in qc.qubits:
            qc.rz(next(ps), q)
    return qc

qc = make(127, 30)

and the not-so-great calling convention

for p in list(qc.parameters):
    qc.assign_parameters({p: Parameter(p.name + "x")}, inplace=True)

Qiskit 1.1.2 took about 60ms, Qiskit 1.2.4 took about 1.1s, and this commit takes about 50ms. This commit is dominated by the Python-space calls dealing with Parameter objects.

This is probably stable for backport, but since the immediate problem the regression was causing was mitigated by other means, and since it substantially touches a core method, I'd propose leaving this til 1.3.

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 11630513281

Details

• 16 of 17 (94.12%) changed or added relevant lines in 2 files are covered.
• 17 unchanged lines in 6 files lost coverage.
• Overall coverage decreased (-0.02%) to 88.718%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/quantumcircuit.py	13	14	92.86%

Files with Coverage Reduction	New Missed Lines	%
crates/accelerate/src/two_qubit_decompose.rs	1	92.09%
crates/accelerate/src/unitary_synthesis.rs	1	92.2%
crates/qasm2/src/expr.rs	1	94.02%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	2	58.05%
crates/qasm2/src/lex.rs	6	91.48%
qiskit/synthesis/two_qubit/xx_decompose/decomposer.py	6	90.77%

Totals
Change from base Build 11617024653:	-0.02%
Covered Lines:	76356
Relevant Lines:	86066

---

💛 - Coveralls

[kevinhartman]

This looks mostly good to me, but it might be nice if you could add a bit more explanation around create_mapping_view in the form of a code comment(s). Personally, I'm not used to seeing nonlocal and I had to look up how it works / its lifetime implications.

[jakelishman]

nonlocal is needed because Python has implicit variable declarations on assignment. If there's no assignment, you do regular scoped lookups. Since we assign to mapping_view in the function, that would imply that we were creating a new variable (doesn't matter that we read it before we wrote to it - that just results in an UnboundLocalError). nonlocal suppresses the implicit-declaration behaviour and instead makes the scoped lookup work as expected, though with the minor restriction that the binding must resolve at compile time to a variable in a nonlocal scope:

x = 0

def f():
    # This is a syntax error because there's no enclosing scopes containing `f`.
    # (The global scope isn't "enclosing" in Python terminology).
    nonlocal x

def g():
    x = 1
    def f():
        # This is fine, and gets the `x = 1`
        nonlocal x
        
class A:
    x = 2
    def f(self):
        # Also a syntax error, because class bodies don't introduce enclosing scopes.
        nonlocal x

Unless you feel particularly strongly, I'd rather not add comments on how nonlocal works - it's not a particularly common keyword, but we do use it occasionally for things just like this, and it's one of the reasons the keyword exists.

[kevinhartman]

@jakelishman

Unless you feel particularly strongly, I'd rather not add comments on how nonlocal works - it's not a particularly common keyword, but we do use it occasionally for things just like this, and it's one of the reasons the keyword exists.

I'm in agreement with this, and I like the way you've described what's going on here in terms of "business logic" (I'd personally never argue to add commentary intended to teach the reader the specifics of a programming language heh).

Thanks for these changes, it'll be a lot more understandable now what's going on here to someone unfamiliar with this module.