Fix incorrect graph construction in VF2PostLayout #9058
Conversation
The `add_nodes` call should be outside the loop; inside, it adds nodes quadratically. This commit also squashes a minor inefficiency in set allocation.
jakelishman
added
stable backport potential
|
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:
|
There was a problem hiding this comment.
LGTM, thanks for fixing this. My first reaction was a test would be good, but there was never actually a correctness issue here with this bug except potentially in the case of an all 1q operation circuit that was running vf2postlayout without vf2 layout (and then only based on random seeding). Which seems unlikely enough that guarding against it happening again doesn't provide much value in practice.
Pull Request Test Coverage Report for Build 3378784841
💛 - Coveralls |
The `add_nodes` call should be outside the loop; inside, it adds nodes quadratically. This commit also squashes a minor inefficiency in set allocation. (cherry picked from commit 61ca9ca)
The `add_nodes` call should be outside the loop; inside, it adds nodes quadratically. This commit also squashes a minor inefficiency in set allocation. (cherry picked from commit 61ca9ca) Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Summary
The
add_nodescall should be outside the loop; inside, it adds nodes quadratically. This commit also squashes a minor inefficiency in set allocation.Details and comments
I couldn't immediately think of a sensible test for this, since the internal graph isn't really accessible.