-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compose multiple final_layout attributes in routing #11399
Compose multiple final_layout attributes in routing #11399
Conversation
This commit adds functionality to the built-in routing passes for composing an existing final layout with the routing permutation. In PRs Qiskit#9523 and Qiskit#11387 we're adding new passes that introduce a permutation prior to routing that needs to be tracked for the final output layout to be valid. To faciliate this a new method compose() is added to the Layout class which will combine the two layouts. Then the built-in routing passes are updated to call compose when a layout has already been set.
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8448544750Warning: 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.
Details
💛 - Coveralls |
While I understand the reason for this PR, I got quite confused about the terminology. A layout is an injective map from virtual qubits to physical qubits, i.e. a function If And I am also confused why the method |
Yeah, I agree with Sasha that using Sasha: the full story here though is that the
Each of the 2-tuples The aim of this function is to produce a third edit: originally I wrote the footnote paragraph in the middle, but it's basically just agreeing with Sasha's terminology, so it's just noise.1 Footnotes
|
I haven't actually reviewed the maths in the PR to check that what I said is what the PR actually achieves, but it's my understanding of what Matt was trying to do with combining the passes. |
UPDATE: ignore this post, it's all wrong Thanks Jake, your explanation makes perfect sense. This is how I am thinking about all this right now: At each point of the computation, we keep a track of the "current" layout Hence, if we first apply (This does not say anything new, just slightly restates what Jake has written). Trying to check the maths, the code currently reads:
If I read this correctly, given a virtual qubit The other question on the terminology is whether in
and it seems that we are updating the layout for |
Sasha: just flicking through the PR quickly now, I'm not certain that it actually is passing With that in mind, I think that if |
To add: because of that, in the current state of the PR, |
Alternatively, if we don't want to tackle changing how |
After an offline discussion with @mtreinish and @sbrandhsn, I finally understand the terminology. Personally, I find it a bit confusing and would have named some things a bit differently, but the intent behind it makes perfect sense. A Note: I am not 100% sure about the following, but when reasoning about the maps Additionally, what makes the code a bit confusing is that internally a
Note that the list of Going back to the previous discussion (that I got all wrong), intuitively the function that composes layouts is the usual composition: given BTW, this is all quite extensively documented in the class description and the function docstrings, but it's very easy to get confused. |
This mostly looks good to me, thanks for looking into this! :-) The only issue I can see is about the requirements of the new methods; if the layout does not represent what the new methods assume, the output has no semantics. I think here we should:
I understand that checking the input assumptions might be as runtime-costly as executing the corresponding method. In these cases I think 2. would be preferable or we should these methods as private methods and only provide public methods that have these checks included... Do you think it makes sense to open a new issue or a separate PR for expanding the documentation you found confusing with examples or similar? On hindsight, these are minor points that can be improved on a follow-up PR. |
Summary
This commit adds several usability methods to the
Layout
class. The methodcompose
is used to compose two layouts together, the methodinverse
is used to find the inverse of a layout, and the methodto_permutation
is used to create a permutation corresponding to the layout.This commit also adds functionality to the built-in routing passes for composing an existing final layout with the routing permutation.
In PRs #9523 and #11387 we're adding new passes that introduce a permutation prior to routing that needs to be tracked for the final output layout to be valid. The compose method introduced here can be used to combine the two layouts.