Fix StochasticSwap routing for if blocks with no else #8891
Conversation
If a control-flow block is not required to be entered (such as an "if" with no "else"), then it can't affect the layout in case the branch isn't taken. This failure would have been clearer in a more control-flow-graph view of the circuit, but we don't yet have that, which is how this bug snuck through. The case of "exit layout need not match entry layout" is really the special case for routing, which this commit makes a bit clearer.
|
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:
|
jakelishman
added
the
stable backport potential
Pull Request Test Coverage Report for Build 3241648236
💛 - Coveralls |
There was a problem hiding this comment.
This LGTM, I think the diff was hard to follow, which just looks like it's caused by things moving around. But the key change seemed to be fairly straightforward as this just combines _route_control_flow_looping and _route_control_flow_multiblock into a single method and does the multiblock handling/optimization inline iff we're evaluating an if/else op with an else block populated (instead of unconditionally for all if/else blocks). This makes sense to me and I didn't catch anything weird looking that I could see in the diff. Plus all the existing tests still pass and we've ramped up the testing quite a bit in recent PRs, so I think this is good to go.
If a control-flow block is not required to be entered (such as an "if" with no "else"), then it can't affect the layout in case the branch isn't taken. This failure would have been clearer in a more control-flow-graph view of the circuit, but we don't yet have that, which is how this bug snuck through. The case of "exit layout need not match entry layout" is really the special case for routing, which this commit makes a bit clearer. (cherry picked from commit 35c4b00)
If a control-flow block is not required to be entered (such as an "if" with no "else"), then it can't affect the layout in case the branch isn't taken. This failure would have been clearer in a more control-flow-graph view of the circuit, but we don't yet have that, which is how this bug snuck through. The case of "exit layout need not match entry layout" is really the special case for routing, which this commit makes a bit clearer. (cherry picked from commit 35c4b00) Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
If a control-flow block is not required to be entered (such as an "if" with no "else"), then it can't affect the layout in case the branch isn't taken. This failure would have been clearer in a more control-flow-graph view of the circuit, but we don't yet have that, which is how this bug snuck through. The case of "exit layout need not match entry layout" is really the special case for routing, which this commit makes a bit clearer.
Details and comments
This should also go into 0.22.
The diff looks funkier than it actually is - in reality, the old
_route_control_flow_loopinggot removed, and_route_control_flow_multiblock(with the addition of amaintain_layoutswitch) got inlined into_control_flow_layer_update, since the split no longer made sense.