add controlflow to basic_swap router #8193
Conversation
minor commit minor commit minor commit
layout on measure revamp current_layout tracking minor commit
|
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:
|
Pull Request Test Coverage Report for Build 2914461973
💛 - Coveralls |
|
I ran a benchmark using red-queen to check the effect on circuits which don't have flow control. There were 151 circuits but 30 of those circuits had errors currently with this pr. Ignoring the errored circuits the results were as follows, The orange data is from this pr. On average, |
Co-authored-by: Tan Jun Liang <43441314+poig@users.noreply.github.com>
Co-authored-by: Tan Jun Liang <43441314+poig@users.noreply.github.com>
| sim = Aer.get_backend("aer_simulator") | ||
| in_results = sim.run(qc, shots=4096).result().get_counts() | ||
| out_results = sim.run(cqc, shots=4096).result().get_counts() | ||
| self.assertEqual(set(in_results), set(out_results)) |
There was a problem hiding this comment.
Do we have a utility function to test if a physical circuit is compliant with a given coupling_map? If not, this would be useful in verifying the outputs circuits here.
There was a problem hiding this comment.
For a target (which is superset of the coupling map) we have https://github.com/Qiskit/qiskit-terra/blob/main/test/python/providers/test_backend_v2.py#L43-L53 which I was using to validate the full transpile() pipe line was valid
Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
…ontrolflow/basic_swap
There was a problem hiding this comment.
I added one comment, but I think this PR isn't currently synced with your local state, perhaps - there's some breakpoint statements left, and bits of the continue/break handling left in. I'll wait for you to ping before reviewing again (and similar on stochastic swap).
| The coupling_map is converted to the virtual qubits of the current layout.""" | ||
| new_coupling = layout_transform(self.coupling_map, current_layout) | ||
| _pass = self.__class__(new_coupling, initial_layout=None) |
There was a problem hiding this comment.
I'm really not a fan of co-opting CouplingMap to be over virtual qubits instead of physical qubits (mentioned on the other PR as well). Coupling maps are specifically of physical qubits - it's explicitly in the docstring of the class, and that gives a proper separation of concerns between mutable Layouts and the fixed underlying hardware.
It's difficult for me to reason about the correctness of the logic if the coupling map doesn't represent the actual physical connections, and it feels like it opens lots of opportunities for bugs when converting between the "physical" qubits of the control-flow routing and the outer-circuit routing. In this case, can we keep the coupling map fixed, and start the inner routing from the current layout? That feels much more natural to me, and it uses the classes as intended.
| if cf_layer or (subdag.depth() == 1 and subdag.op_nodes()[0].name == "continue_loop"): | ||
| order = current_layout.reorder_bits(new_dag.qubits) | ||
| new_dag.compose(subdag, qubits=order) | ||
| current_layout = cf_layout | ||
| else: | ||
| order = current_layout.reorder_bits(new_dag.qubits) | ||
| new_dag.compose(subdag, qubits=order) |
There was a problem hiding this comment.
This still has continue-loop handling in it right now.
|
putting this on hold until after #8418 |
|
with #8418 merged, removing |
Summary
This pr adds controlflow handling to the BasicSwap router.
Details and comments
if_else: add swaps to match layouts of both blocks
for, while: add swaps to match final layout to be same as initial layout
improvements on this scheme to come in future prs.
Fixes #9430