-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Improve the ergonomics of the TranspileLayout class #10835
Conversation
This commit updates the TranspileLayout class to make it easier to work with. The TranspileLayout class was orginally just an a container class that took the information returned from the transpiler needed to reverse the transpiler layout permutation (in the absence of ancillas) so that `QuantumCircuit.from_circuit()` could compare Operator instances across a `transpile()`. However, the internal data structures used by the transpiler are hard to work with, and don't map well to the normal reasoning around how the transpiler transforms the circuit. To improve the usability of the class this commit adds 4 new methods to the class: - initial_layout_list() and final_layout_list() which compute a list form of the initial_layout and final_layout respectively - full_layout() which generates a list that maps the input circuits qubit positions in the input circuit to the final position at the end of the circuit (which is what most people think of when they hear "final layout") - permute_sparse_pauli_op() which takes in a SparsePauliOp object and permutes it based on the full layout. This is especially useful when combined with the Estimator primitive To implement these functions a few extra pieces of information are needed to fully implement them. The first is we need to know the number of original circuit qubits. This is used to account for any ancillas that are added in the circuit, once we have the input circuit count we can use the original_qubit_indices attribute to discern which qubits in the initial layout are from the original circuit and which are ancillas. The second piece of information needed is the list of qubits in the output circuit. as this is needed to look up the position of the virtual qubit in the final_layout. These are both added as optional private attributes to the TranspileLayout class and there are some small changes to the pass manager and QPY to accomodate them. Similarly the change in the TranspileLayout class requires a new QPY version to include the missing details that were not being serialized in QPY and weren't representable in the previous payload format. Fixes Qiskit#10826 Fixes Qiskit#10818
One or more of the the following people are requested to review this:
|
This looks like it is more or less what is needed here. However, I am not a big fan of calling the "final layout" the "full layout" as it seems off. Namely in the |
One question too is what happens to a layout if the swap mapper decides to use an ancilla qubit for routing? The active number of qubits is no longer the same as the original circuit. Does this just track the permutations of the original set? |
Well the path to changing
I don't have an issue changing the names of anything added in this PR, |
For
EDIT: I added a test case to show this in: 8546928 |
Building on what's in this PR, maybe something like this works as a full set of coherent operations? I've judiciously used the word "virtual" to avoid naming clashes and distinguish bi-maps class TranspileLayout:
# Bi-map of initial positions (post-layout, pre-routing) of virtual `Qubit`
# to physical `int`
def initial_virtual_layout(self, filter_ancillas: bool) -> Layout: ...
# Map of initial positions virtual list indices to physical `int`
def initial_index_layout(self, filter_ancillas: bool) -> List[int]: ...
# Bi-map of final positions (post-routing) of virtual `Qubit` to physical `int`.
def final_virtual_layout(self, filter_ancillas: bool) -> Layout: ...
# Map of final positions virtual list indices to physical `int`.
def final_index_layout(self, filter_ancillas: bool) -> List[int]: ...
# Potentially this too (I can't see much meaning for `filter_ancillas` here.)
def routing_permutation(self) -> List[int]: ...
# ... and now a bunch of helper methods that could also live elsewhere.
def permute_to_initial(self, operator: OperatorLike) -> OperatorLike: ...
def permute_to_final(self, operator: OperatorLike) -> OperatorLike: ... For comparison:
I know the argument for edit: and just for clarity, it hardly matters much what we actually store as the internal properties of the object if they're not publicly accessible - all the methods are calculable from knowing three pieces of information:
The PR already ensures we have access to all those three things, so there's no trouble calculating everything we need from there. There's many combinations of bits of the information we could store to recalculate everything from. |
I should also say: I like the general direction of the PR, and I think this will be a very nice ergonomic improvement for users of the transpiler across the board - it's not exposing any information they couldn't already calculate themselves, but it's definitely making it easier to work with. |
Pull Request Test Coverage Report for Build 6228373802
💛 - Coveralls |
I'm fine with this as the API we go with here, it makes sense to me and is a lot clearer for users than what I came up with. I agree that The only thing I think missing here is the equivalent of |
I think my issue with the existing I think the closest coherent object we could put out for the current initial = transpile_layout.initial_virtual_layout(filter_ancillas=True)
final = transpile_layout.final_virtual_layout(filter_ancillas=True)
{
virtual: (initial[virtual], final[virtual])
for virtual, initial_physical in initial.get_virtual_bits().items()
} I'm not super sure what use such an object could have though, which is why I didn't include it. In terms of methods vs attributes: personally, I'd just make everything public here a method, even if (as an internal detail), it just returns a copy of an attribute. The cost of copying a layout is small compared to any operation using layouts, and users shouldn't really be able to mutate a |
This commit reworks the names of the new methods added in this API to be more self consistent. It uses the format '*_index_layout' to replace the `*_layout_list` name. Similarly new methods `*_virtual_layout` are added for methods that return layout objects.
I reworked the API per the suggestions in 3f2542b . The only thing I haven't touched is the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the layout-related parts of the API look good and far more ergonomic in this new form, but I'm obviously biased so it would be good to have review on that from other potential users. I'm happy to approve those parts.
I think we should get some input from others on other teams / other downstream consumers of Qiskit about things like permute_sparse_pauli_op
with regard to what methods would be useful to them, and where it would make most sense for them to live, but I think the principle of them is certainly something that's going to be part of a full UI for users.
LAYOUT_V2 = namedtuple( | ||
"LAYOUT", | ||
[ | ||
"exists", | ||
"initial_layout_size", | ||
"input_mapping_size", | ||
"final_layout_size", | ||
"extra_registers", | ||
"input_qubit_count", | ||
], | ||
) | ||
LAYOUT_V2_PACK = "!?iiiIi" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this struct
format string doesn't match the definition of the C struct up top - exists
is a char
in the C definition and _Bool
here - but in practice it would be pretty wild to have a system where _Bool
wasn't ABI compatible with char
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I probably just copy and pasted both from V1 as this doesn't change between V1 and V2. IIRC we added this discrepancy between docs and the struct
format string as part of your review in #10148: #10148 (comment) . I can update it to be _Bool
in the docs if you prefer though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, it's fine, let's just leave it.
|
||
If there is no :attr:`.final_layout` attribute present then that indicates | ||
there was no output permutation caused by routing or other transpiler | ||
transforms. In this case the function will return a list of ``[0, 1, 2, .., n]`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly I guess this would go to n - 1
?
releasenotes/notes/transpile-layout-improvements-118dd902d93e5b96.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jake Lishman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just gave this a review because I am also interested in this topic. I recall having looked into this some months ago when I was using skip_transpilation
with the Estimator
and ran into Qiskit/qiskit-ibm-runtime#338.
Unfortunately, I no longer have the code snippets that I was using at the time. That said, I think the proposed API here makes sense to me and would be reasonable to use (definitely better than how one would need to go about this right now) 👍
As per the permute_sparse_pauli_op
method: I suppose the method here is fine. From experience I can say that I always work with SparsePauliOp
and no other operator classes. Whether you need to support permuting other objects is up to you and might be better discussed in terms of API consistency within the quantum_info
module.
FWIW a while ago I implemented a similar index permutation utility in the SparseLabelOp
classes in Qiskit Nature: qiskit-community/qiskit-nature@d9599dd
Rather than relying on a composition with a permuted identity operator (as you do here) I chose to act directly on the underlying labels. Not sure if this is "better" but it might be easier to achieve especially thinking about the underlying PauliList
which are just some numpy arrays that natively support axis permutations.
I will improve |
I played around with this and it works quite well. |
This commit remove the TranspileLayout.permute_sparse_pauli_op() method. This was a bit out of place on the TranspileLayout class, and it will instead be replaced in a follow-up PR by a apply_layout() method on the SparePauliOp class that takes in a TranspileLayout object.
I've updated this PR to not have the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw a couple of things in the documentation, but otherwise this looks good to me.
It makes more sense to me too to have the permute method on the operators, though we can always revisit it in the future; it's of course easier to add methods than to remove ones we don't like.
* Improve the ergonomics of the TranspileLayout class This commit updates the TranspileLayout class to make it easier to work with. The TranspileLayout class was orginally just an a container class that took the information returned from the transpiler needed to reverse the transpiler layout permutation (in the absence of ancillas) so that `QuantumCircuit.from_circuit()` could compare Operator instances across a `transpile()`. However, the internal data structures used by the transpiler are hard to work with, and don't map well to the normal reasoning around how the transpiler transforms the circuit. To improve the usability of the class this commit adds 4 new methods to the class: - initial_layout_list() and final_layout_list() which compute a list form of the initial_layout and final_layout respectively - full_layout() which generates a list that maps the input circuits qubit positions in the input circuit to the final position at the end of the circuit (which is what most people think of when they hear "final layout") - permute_sparse_pauli_op() which takes in a SparsePauliOp object and permutes it based on the full layout. This is especially useful when combined with the Estimator primitive To implement these functions a few extra pieces of information are needed to fully implement them. The first is we need to know the number of original circuit qubits. This is used to account for any ancillas that are added in the circuit, once we have the input circuit count we can use the original_qubit_indices attribute to discern which qubits in the initial layout are from the original circuit and which are ancillas. The second piece of information needed is the list of qubits in the output circuit. as this is needed to look up the position of the virtual qubit in the final_layout. These are both added as optional private attributes to the TranspileLayout class and there are some small changes to the pass manager and QPY to accomodate them. Similarly the change in the TranspileLayout class requires a new QPY version to include the missing details that were not being serialized in QPY and weren't representable in the previous payload format. Fixes Qiskit#10826 Fixes Qiskit#10818 * Handle None for the private TranspileLayout fields in QPY * Add test for full_layout with ancillas * Rework interface to be more consistent This commit reworks the names of the new methods added in this API to be more self consistent. It uses the format '*_index_layout' to replace the `*_layout_list` name. Similarly new methods `*_virtual_layout` are added for methods that return layout objects. * Apply suggestions from code review Co-authored-by: Jake Lishman <[email protected]> * Remove permute_sparse_pauli_op() This commit remove the TranspileLayout.permute_sparse_pauli_op() method. This was a bit out of place on the TranspileLayout class, and it will instead be replaced in a follow-up PR by a apply_layout() method on the SparePauliOp class that takes in a TranspileLayout object. * Update docs * Fix docs build post-rebase * Fix docs issues --------- Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: Matthew Treinish <[email protected]>
* Refactor internals of pass manager and flow controllers. This PR introduces two changes for * remove tight coupling of flow controller to pass runner instance, * remove pass normalization. PropertySet becomes context var so that flow controllers can be instantiated without coupling to a pass runner instance. BasePass becomes an iterable of length 1 to skip normalization. The decoupling of property set from pass runner allows pass manager to directly broadcast pass runner in the multithread, rather than distributing self and craete multiple pass runner in each thread. * Replace class attribute PASS_RUNNER with property method. This allows subclass to dispatch different pass runner type depending on the target code. * Refactor passmanager module - Add OptimizerTask as a protocol for the pass and flow controller. These are in principle the same object that inputs and outputs IR with optimization. - A task gains execute method that takes IR and property set. This makes property set local to iteration of the passes. - Drop dependency on pass runner. Now pass manager has a single linear flow controller. - Pass manager gain responsibility of compiler frontend and backend as pass runner dependency is removed. This allows flow controllers to be still type agnostic. - Drop future property set, as this is no longer necessary because optimization task has the execute method explicitly taking the property set. * Refactor transpiler passmanager - Remove pass runner baseclass and replace RunningPassmanager baseclass with FlowControllerLiner - Implemented frontoend and backend functionality in transpiler Pass manager * Rename module: base_pass -> base_optimization_tasks * Backward compatibility fix - Move handling of required passes to generic pass itself. This makes optimizer tasks the composite pattern-like for more readability. - Readd MetaPass to transpiler BasePass as a metaclass which implements predefined equivalence check for all passes. This is necessary to take care of duplicated pass run, which is prohibited in circuit pass manager. - Add FlowControllerMeta that allows users to subclass FlowController, while delegating the pass control to BaseFlowController. - Readd count to pass manager callback - Add PassState that manages the state of execution including PropertySet. This is a portable namespace that can be shared across passes. * Update module documentation * Update release note * lint fix * OptimizationTask -> Task In multi-IR realm task can be something other than optimization. For example IR conversion. Name should not limit operation on subclass. * Typo fix FlowControllerLiner -> FlowControllerLinear * Use RunState value. Set 0 to success case. * Separate property set from pass state. Now Task.execute method has two arguments for property set and pass state. Lifetime of property set data is entire execution of the pass manager, while that of pass state is execution of a single pass. * Pending deprecate fenced_property_set and remove usage. * Pending deprecate FencedObject baseclass and remove usage. * Add some inline comments. * Convert RunState to raw Enum * Change the policy for the use of multiple IRs. Just updated module documentation, no actual code change. Future developer must not introduce strict type check on IR (at least in base classes in the module). * Updated interface of base controller. Namely .append() and .passes are moved to particular subclass and the baseclass is now agnostic to construction of task pipeline. This adds more flexibility to design of conditioned pipelines. Note that PassState is also renamed to WorkflowStatus because this is sort of global mutable variable, rather than local status information of a particular pass. * Remove FlowControllerMeta and turn FlowController into a subclass of BaseController. * Remove dependency on controller_factory method and deprecate. * Update release note * Readd code change in #10835 Co-authored-by: Matthew Treinish <[email protected]> * typehint fix * doc fix * move FencedPropertySet class back to transpiler.fencedobjs * Temporary: add lint ignore * Fix example code in class doc * Tweaks to documentation * Change baseclass of the MetaPass and revert f28cad9 * Update interface of execute and use generator feature. - Add new container class PassmanagerMetadata - Rename propertyset with compilation_status - Provide metadata with the iter_tasks instead of property_set - Send metadata through generator - Turn metadata required, and let pass manager create metadata - Return metadata from execute along with the IR * Update deprecations * Delay instantiation of FlowControllerLinear until execution of pass manager * Stop wrapping a list of tasks with flow controller linear. * Remove flow_controller_conditions from base controller append and add stacklevel to deprecation warning * Misc updates * disable cyclic-import * Remove typecheck * Rename `PassmanagerMetadata` to `PassManagerState` This is primarily to avoid a potential conflict with the terminology `metadata` that's associated with the `input_program`, and because the object is the state object for a pass-manager execution. There is still a risk of confusion with `RunState`, but since that's a subcomponent of `PassManagerState`, it feels fair enough. * Correct warning stacklevel --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Jake Lishman <[email protected]>
Summary
This commit updates the TranspileLayout class to make it easier to work with. The TranspileLayout class was orginally just a container class that took the information returned from the transpiler needed to reverse the transpiler layout permutation (in the absence of ancillas) so that
QuantumCircuit.from_circuit()
could compare Operator instances across atranspile()
. However, the internal data structures used by the transpiler are hard to work with, and don't map well to the normal reasoning around how the transpiler transforms the circuit. To improve the usability of the class this commit adds 4 new methods to the class:permute_sparse_pauli_op() which takes in a SparsePauliOp object and permutes it based on the full layout. This is especially useful when combined with the Estimator primitiveThis was removed and will be added in a follow-up PR asSparsePauliOp.apply_layout()
: Addapply_layout
method toSparsePauliOp
#10947To implement these functions a few extra pieces of information are needed to fully implement them. The first is we need to know the number of original circuit qubits. This is used to account for any ancillas that are added in the circuit, once we have the input circuit count we can use the original_qubit_indices attribute to discern which qubits in the initial layout are from the original circuit and which are ancillas. The second piece of information needed is the list of qubits in the output circuit. as this is needed to look up the position of the virtual qubit in the final_layout. These are both added as optional private attributes to the TranspileLayout class and there are some small changes to the pass manager and QPY to accomodate them.
Similarly the change in the TranspileLayout class requires a new QPY version to include the missing details that were not being serialized in QPY and weren't representable in the previous payload format.
Details and comments
Fixes #10826
Fixes #10818