Found this while profiling operator arithmetic on large QubitOperators.
SymbolicOperator.__add__, __sub__, __mul__ and accumulate all copy the
operand with copy.deepcopy and then mutate the copy in place. But a term key
is a tuple of (index, action) tuples and a coefficient is an int, float,
complex or sympy expression, all immutable, so the recursive copy buys nothing:
the in-place operators only rebind self.terms[term] and add or remove keys,
they never mutate a key or coefficient object. A plain dict(self.terms) gives
the same out-of-place guarantee.
The gap is large. Random QubitOperators over 30 qubits:
terms op main patched
5000 A + B 85.8 ms 2.5 ms
20000 A + B 348 ms 11.8 ms
20000 2.0 * A 339 ms 4.7 ms
Medians over 25 repetitions, two alternated main/patch rounds with fixed seeds,
on an Intel Core Ultra 5 225 laptop.
I have a patch ready: a small _clone helper (empty instance plus a copy of
the terms dict) used in the four call sites. The existing out-of-place tests
pass, black and mypy are clean, and every changed line is covered by the
current suite.
Opening a pull request currently appears to be limited to collaborators, so I'm
filing this instead; happy to send the patch over if wanted. It came out of a
wider look at the operator hot paths, and I have a couple more findings queued.
Found this while profiling operator arithmetic on large QubitOperators.
SymbolicOperator.__add__,__sub__,__mul__andaccumulateall copy theoperand with
copy.deepcopyand then mutate the copy in place. But a term keyis a tuple of
(index, action)tuples and a coefficient is an int, float,complex or sympy expression, all immutable, so the recursive copy buys nothing:
the in-place operators only rebind
self.terms[term]and add or remove keys,they never mutate a key or coefficient object. A plain
dict(self.terms)givesthe same out-of-place guarantee.
The gap is large. Random QubitOperators over 30 qubits:
Medians over 25 repetitions, two alternated main/patch rounds with fixed seeds,
on an Intel Core Ultra 5 225 laptop.
I have a patch ready: a small
_clonehelper (empty instance plus a copy ofthe terms dict) used in the four call sites. The existing out-of-place tests
pass, black and mypy are clean, and every changed line is covered by the
current suite.
Opening a pull request currently appears to be limited to collaborators, so I'm
filing this instead; happy to send the patch over if wanted. It came out of a
wider look at the operator hot paths, and I have a couple more findings queued.