Qiskit / qiskit / 27417302793

87%

main: 88%

Build Type

push

github

Committed by web-flow

Commit Message

Move `ParameterVector` to Rust (#16228)

* Move `ParameterVector` to Rust

`ParameterVector` was previously a pure-Python concept, even though the
Rust-space `Symbol` necessarily had to track an internal backreference
to a vector Python object in order to convert itself to Python later.
This hidden dependence on the Python interpreter was the root cause of a
panicking bug in Qiskit 2.4[^1].

All `ParameterVectorElement`s in Python space have names and UUIDs that
are pure functions of the underlying vector and the element's index.
Now that this information is available in Rust space, it is far more
efficient to simply not store all this derived information, but
calculate it on the fly, as needed.  This motivated the change in
`Symbol` to an `enum`: the logic was _already_ handling the two cases of
"standalone" and "element", but this formally separates them to avoid
storing unnecessary data.  Doing this alone is already a performance
benefit: construction of a `list(ParameterVector("a", 10_000))` (to
ensure all the same Python-space objects are created) went from 7.7ms
to 1.4ms on my machine (~5x speedup).

This work here is far more aggressive at threading through shared `Arc`
references of both `Symbol` and `SymbolVector`.  The `Arc<SymbolVector>`
is necessary for correctness in Python space, not just performance: we
must have `all(el.vector == els[0].vector for el in els)`, and we don't
want to have to cache more Python objects within `Symbol` to achieve
that.  Unfortunately, `ParameterVector` never implemented true equality
(only the default referential equality that Python uses to make
mutable-state objects safely hashable by default), so we certainly need
some other stable reference that can be shared.

The previous Python-space constructions of `PyParameter` and
`PyParameterVectorElement` had not been done entirely correctly with
respect to subclassing; the same (cloned) `Symbol` was present in
multiple places, which was two more allocations tha... (continued)

Coverage Stats

326 of 410 new or added lines in 23 files covered. (79.51%)

14 existing lines in 4 files now uncovered.

110769 of 126618 relevant lines covered (87.48%)

965641.04 hits per line