qiskit
qiskit copied to clipboard
Qiskit
Support standalone `expr.Var` in QPY
Summary
This necessitates adding some extra definitions to the circuit header. When standalone variables are used, we do not re-encode the entire variable (which would take a miniumum of ~20 bytes per variable), but instead just emit an index into the order that the variables were defined at the top of the circuit.
At present, each control-flow scope will store the Var nodes anew, so it's not a perfect only-store-once system. This is consistent with how registers are handled, though, and the QPY format isn't particularly memory optimised as things stand.
Details and comments
I need to write a couple more tests, including control-flow scopes, but this is the system in principle. Equality tests are stymied by not having access to QuantumCircuit.__eq__ with Var because of the lack of support in DAGCircuit.
Close #10930.
One or more of the the following people are requested to review this:
-
@Qiskit/terra-core -
@mtreinish -
@nkanazawa1989
Pull Request Test Coverage Report for Build 8893323077
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 123 of 127 (96.85%) changed or added relevant lines in 6 files are covered.
- 986 unchanged lines in 82 files lost coverage.
- Overall coverage increased (+0.1%) to 89.483%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| qiskit/qpy/type_keys.py | 11 | 15 | 73.33% |
| <!-- | Total: | 123 | 127 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| qiskit/circuit/library/standard_gates/r.py | 1 | 97.62% |
| qiskit/transpiler/target.py | 1 | 93.74% |
| crates/qasm2/src/expr.rs | 1 | 94.03% |
| qiskit/primitives/backend_sampler_v2.py | 1 | 99.21% |
| qiskit/circuit/library/generalized_gates/permutation.py | 1 | 93.1% |
| qiskit/primitives/containers/data_bin.py | 1 | 97.92% |
| qiskit/circuit/library/standard_gates/rxx.py | 1 | 97.44% |
| qiskit/circuit/library/standard_gates/u2.py | 1 | 96.67% |
| qiskit/circuit/library/standard_gates/rzx.py | 1 | 97.44% |
| qiskit/quantum_info/operators/operator.py | 1 | 94.94% |
| <!-- | Total: | 986 |
| Totals | |
|---|---|
| Change from base Build 8653934653: | 0.1% |
| Covered Lines: | 61093 |
| Relevant Lines: | 68273 |
💛 - Coveralls
This is being deferred to 1.1 along with the rest of the Var and Store handling. The PR as written depends on #11666 for its tests to pass, because the last test triggers the bug described in that PR.
Huh, I forgot I'd already made this PR.
Anyway, I rebased this branch over main, cherry-picked in #11820 and #11666 which are required, and I'll rebase them out again once they're merged.