Qiskit
Prepare `PauliEvolutionGate` for Rustiq & port it to Rust
Summary
Port the PauliEvolutionGate synthesis to Rust, plus, expose the Pauli network and allow other plugins to synthesize the gate. Also adds the plugin structure for the gate for #12789.
Details and comments
The larger the Pauli network to synthesis, the better the speedup from the port to Rust. Here I measured a Heisenberg Hamiltonian (XX+YY+ZZ on interacting qubits, plus 1-qubit Z on each qubit) on a square lattice for different settings:
1 timestep for first order Trotter: speedup @ 100 qubits is 2.8
N=25: 7.0
main: 0.007 +- 0.006
this: 0.001 +- 0.000
N=100: 2.8
main: 0.042 +- 0.000
this: 0.015 +- 0.000
N=255: 2.8
main: 0.198 +- 0.040
this: 0.071 +- 0.003
N=400: 2.3
main: 0.607 +- 0.001
this: 0.260 +- 0.000
10 timestep for 4th order Trotter: speedup @ 100 qubits is 9.4
N=25: 6.7
main: 0.040 +- 0.003
this: 0.006 +- 0.000
N=100: 9.4
main: 0.330 +- 0.037
this: 0.035 +- 0.001
N=255: 10.8
main: 1.291 +- 0.066
this: 0.119 +- 0.007
N=400: 11.5
main: 4.052 +- 0.051
this: 0.353 +- 0.009
1 timestep for first order Trotter but with wrap=True: speedup @ 100 qubits is 3.6
N=25: 3.4
main: 0.034 +- 0.012
this: 0.010 +- 0.004
N=100: 3.6
main: 0.187 +- 0.017
this: 0.052 +- 0.010
N=255: 3.1
main: 0.551 +- 0.048
this: 0.178 +- 0.019
N=400: 3.0
main: 1.367 +- 0.068
this: 0.458 +- 0.027
Side effects of this PR:
- add
SparsePauliOp.to_sparse_listto construct the sparse list format (i.e.op == SparsePauliOp.from_sparse_list(op.to_sparse_list())) - refactor
LieTrotterto just be an alias ofSuzukiTrotterand reduce code duplication - fix the docstrings to use actually non-commuting Paulis (XX and ZZ commute and would have no expansion error)
Pull Request Test Coverage Report for Build 11667384085
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
- 313 of 332 (94.28%) changed or added relevant lines in 12 files are covered.
- 587 unchanged lines in 28 files lost coverage.
- Overall coverage increased (+0.03%) to 88.756%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| crates/circuit/src/operations.rs | 1 | 2 | 50.0% |
| qiskit/synthesis/evolution/suzuki_trotter.py | 18 | 20 | 90.0% |
| qiskit/synthesis/evolution/product_formula.py | 41 | 45 | 91.11% |
| qiskit/synthesis/evolution/qdrift.py | 14 | 18 | 77.78% |
| crates/accelerate/src/circuit_library/pauli_evolution.rs | 204 | 212 | 96.23% |
| <!-- | Total: | 313 | 332 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| qiskit/transpiler/target.py | 1 | 93.84% |
| qiskit/circuit/library/boolean_logic/quantum_or.py | 1 | 98.08% |
| crates/accelerate/src/basis/basis_translator/basis_search.rs | 1 | 99.31% |
| qiskit/circuit/library/boolean_logic/quantum_and.py | 1 | 97.96% |
| qiskit/circuit/library/boolean_logic/inner_product.py | 1 | 96.0% |
| qiskit/circuit/store.py | 1 | 97.06% |
| qiskit/qasm2/export.py | 1 | 98.48% |
| crates/accelerate/src/split_2q_unitaries.rs | 2 | 96.43% |
| qiskit/transpiler/passmanager_config.py | 3 | 96.1% |
| crates/accelerate/src/basis/basis_translator/compose_transforms.rs | 3 | 97.48% |
| <!-- | Total: | 587 |
| Totals | |
|---|---|
| Change from base Build 11601286937: | 0.03% |
| Covered Lines: | 76579 |
| Relevant Lines: | 86280 |
💛 - Coveralls
One or more of the following people are relevant to this code:
-
@Cryoris -
@Qiskit/terra-core -
@ajavadia
Hmm, after switching SX and SXdg in 2b6f4eb, some of the tests in pauli_feature_map now fail. I believe the tests were incorrect before 2b6f4eb, so it's good that they would be fixed, yet it's scary how difficult it is to distinguish between the correct and incorrect behaviors (I was actually looking at these before and they looked perfectly fine to me). Can we add more Operator-based asserts? I.e. in addition to checking that the ref circuit looks as it's supposed to, also that Operator(encoding) == Operator(ref)?
Other than that, I am very happy with this PR, thanks for the great work!
For the record, I am no longer sure what was our conclusion with @raynelfss (Ray is so ahead of me when discussing Rust types). I know that @Cryoris used boxing to be able to return both the usual and the double-ended iterators (and maybe something else too), yet if I remember correctly the double-ended iterators are a bit heavier than the usual ones. Does it make sense for us to return the double-ended iterators everywhere? Another possibility is maybe we should return the CircuitData objects (instead of Vec<Instruction> or iterator over that).
In hindsight @Cryoris, we might not be able to use a simple impl Trait I believe I can now see why you used so many Box pointers.
This is because this function returns different types of Iterator depending on the input (e.g. Chain<Map<...>> vs Empty). The dynamic type was the only way I got to work, but I'm happy to change it if there's a better way 🙂
Even though both Once iterators and an example iterator such as Chain<Map<Iterator>> should technically both count as Iterator the types are different enough to require a dynamic block, and so you'd need to use the Box there. I can't think of another way to do this right now, so you can keep the Box here.
