Add NLayout::compose method #14783
Add NLayout::compose method #14783
Conversation
This commit adds a method to compose two NLayout objects in rust, this is something that we'll need when we do a full path compilation in Rust as we'll be working directly with NLayout and we'll potentially need to compose layouts depending on the passes that run.
mtreinish
added
Changelog: None
|
One or more of the following people are relevant to this code:
|
| .iter() | ||
| .enumerate() | ||
| .for_each(|(virt_idx, phys)| { | ||
| new_v2p[virt_idx] = other_v2p[phys.index()]; |
There was a problem hiding this comment.
It shouldn't be meaningful to index v2p using a physical qubit. Something is off here with the definition of composition and/or what the qubits represent.
There was a problem hiding this comment.
[to the] physical Q-qubits, and the other layout is a mapping from virtual Q-qubits to physical R-qubits
This part of the comment doesn't make sense. It's implying there's a hidden additional layout mapping between the physical qubits of self and the virtual qubits of other which is the indexwise identity. That largely violates what a virtual qubit is: it implies the second NLayout object is not a true layout mapping one space to another, but instead a permutation within the physical-qubit space.
There was a problem hiding this comment.
That's fair the composition here that I need is really just for the routing permutation which I've been representing with Vec<PhysicalQubit>. I only put this in NLayout because I didn't really know where else to put this and we had the equivalent method in python's layout. But I agree it's not actually a good fit for the NLayout model I probably can just make this a standalone function for Vec<PhysicalQubit> and the only reason it's in python is because we're stuck using Layout for the permutation tracking.
There was a problem hiding this comment.
I think it's fine on NLayout if that's convenient for the final layout, but let's just have it take a mapping [old] PhysicalQubit -> [new] PhysicalQubit (either as a slice or a generic over a function, or whatever) rather than another bidirectional layout object, so the mathematics is sound.
There was a problem hiding this comment.
Writing the comment here made me realize this doesn't actually solve the problem I was intending to, since I was planning to keep routing_permutation as a Vec<PhysicalQubit> in TranspileLayout. Maybe the answer is to add TranspileLayout::compose_routing_permutation() instead since that's really what we need here. The goal of this was to have something to compose the permutations from ElidePermutations, Split2qUnitaries, and sabre for the return TranspileLayout.
Pull Request Test Coverage Report for Build 16452886889Details
💛 - Coveralls |
|
I'm going to close this and just embed this into #14778 |
Summary
This commit adds a method to compose two NLayout objects in rust, this is something that we'll need when we do a full path compilation in Rust as we'll be working directly with NLayout and we'll potentially need to compose layouts depending on the passes that run.
Details and comments