qiskit
qiskit copied to clipboard
Qiskit
Add control-flow support to "properties" transpiler passes
Summary
This adds control-flow support to the DAGCircuit methods depth and size, to QuantumCiruit.depth, and to the transpiler pass ContainsInstruction. Together, these are the basis of the principal transpiler passes that calculate properties of circuits for other passes.
Depth and size are not fully defined for control-flow operations, so these methods return hints to the value.
Details and comments
This still needs tests, and to be honest, it may be more appropriate to split it into separate PRs. It passes my informal testing already.
edit: This now has its tests and is ready. It may still be more appropriate to split it into separate PRs.
Thank you for opening a new pull request.
Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.
While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.
One or more of the the following people are requested to review this:
- @Qiskit/terra-core
Pull Request Test Coverage Report for Build 3153488928
- 70 of 72 (97.22%) changed or added relevant lines in 8 files are covered.
- 42 unchanged lines in 4 files lost coverage.
- Overall coverage decreased (-0.04%) to 84.51%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| qiskit/dagcircuit/dagcircuit.py | 46 | 48 | 95.83% |
| <!-- | Total: | 70 | 72 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| qiskit/extensions/quantum_initializer/squ.py | 2 | 79.78% |
| src/optimize_1q_gates.rs | 2 | 93.55% |
| src/dense_layout.rs | 4 | 91.89% |
| src/results/marginalization.rs | 34 | 68.97% |
| <!-- | Total: | 42 |
| Totals | |
|---|---|
| Change from base Build 3153321152: | -0.04% |
| Covered Lines: | 60792 |
| Relevant Lines: | 71935 |
💛 - Coveralls
Choosing the "minimal depth" means the number isn't suitable for any of the places we actually use it - it would allow termination of the optimisation loops without considering the longer branches or while loops. You're of course right that we can't return an accurate number, but the general "depth" is "length of the critical path", which is more logically the longer number. We can't fully account for how many iterations a while loop will be in its worst case (infinite), but for "depth" to have meaning, we do need to take them into account somehow.
I can update the documentation to more clearly stress that it's just a hint to depth rather than an actual value, if you think that's better?
Updated with comments from the dynamic-circuits meeting on Monday, and ready for final review.
Updated with Kevin's comments and ready for review again. For top-level visibility: offline, we decided to remove the changes to QuantumCircuit from this iteration, including not making the QuantumCircuit properties methods produce errors when control-flow is present. We can update these methods later, potentially for the next release, because it's not super clear what exactly they should return for users.