Skip to content
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

Publically expose QuantumCircuit._layout and add helper functions for working with embedded layout #8803

Closed
mtreinish opened this issue Sep 28, 2022 · 8 comments · Fixed by #9486
Labels
type: feature request New feature or request
Milestone

Comments

@mtreinish
Copy link
Member

What should we add?

As discussed in #8800 the way we're embedding the Layout from the transpiler into the output quantum circuit is a bit of a weird place right now. We have a private attribute _layout which is used widely across the qiskit code but we provide no public or supported interface for working with this. This came to be an issue for fixing #8800 because to fix the issue we changed what was stored in the _layout attribute, and while it is a private layout external users depend on it because it was the only way to reason about the permutation caused by initial layout in the transpiler. To address this gap in 0.23.0 we should introduce a public QuantumCircuit.layout attribute and/or also have methods to work with the embedded layout. Like something to return the qargs for the circuit.

@mtreinish mtreinish added the type: feature request New feature or request label Sep 28, 2022
@mtreinish mtreinish added this to the 0.23.0 milestone Sep 28, 2022
@mtreinish mtreinish modified the milestones: 0.23.0, 0.24.0 Jan 9, 2023
@kdk
Copy link
Member

kdk commented Jan 18, 2023

I think QuantumCircuit.metadata is a more natural place to include this information (since a Layout itself isn't strictly a property of a single circuit, it's a mapping of (virtual) qubits in one circuit to (physical) qubits in another).

Another piece it would be good to discuss before we finalize this direction is whether or not to include pre/post Permutations explicitly as a result of the layout and routing passes (with an optimization pass to move them after of qubit initialization and before qubit measurement). This had been discussed in the past and had some advantages in e.g. transpiling subcircuits, but I don't think there was ever a concrete plan of implementation.

@jakelishman
Copy link
Member

I'm not convinced that metadata is right, and I think some of that is that I don't entirely agree with the idea that the "physical" qubits are in a different circuit - they're specific to a machine, but then so are the operations that are contained within the circuit. I do buy the idea that the mapping isn't really an property of the circuit in a vacuum, but a transpiled circuit already doesn't exist in one.

I'm nervous about us suddenly applying important meaning to metadata, because the getter documentation for it says:

The metadata for the circuit is a user provided dict of metadata for the circuit. It will not be used to influence the execution or operation of the circuit, but it is expected to be passed between all transforms of the circuit (ie transpilation) and that providers will associate any circuit metadata with the results it returns from execution of that circuit.

Putting the mapping to hardware qubits in that dictionary seems to me to be a violation of the idea that metadata doesn't influence execution or operation of the circuit. We'd also be newly adding a restriction on the keys available for users to assign with arbitrary data, and I really don't like how using dict directly masks the typing information that IDEs / linters would be able to use if it's a regular attribute.

(Technically there's typing syntax to mitigate some of that last point, if you're prepared to reserve a key in metadata that users can never use, but I personally think that's a hack around bad coding practices in Python.)

@jakelishman
Copy link
Member

All that's to say that my vote is to use a public .layout attribute.

@mrossinek
Copy link
Member

I would like to add a comment here to explain an observation that I made a few weeks ago. Specifically, I was working with the primitives and wanted to skip transpilation on the server side. This meant, that I had to transpile my circuit locally and set the corresponding option in my primitive job.

However, this immediately led to the problem that my (now physical) circuit contained a different number of qubits than my operator resulting in errors (obviously). I could not find a builtin way of how to deal with this situation (that is to say, I did not find a transformation method which would map my qubit operator to the correct set of physical qubits) so I ended up using the private _layout attribute of my QuantumCircuit to build a simple expansion routine for my operator.

There must be some similar code running somewhere on the server side but I did not find that.

All that is to say:

  • I would also be in favor of some public layout information
  • I would also argue a builtin way for what I had to do above should be provided based on the public layout information

@jakelishman
Copy link
Member

jakelishman commented Jan 27, 2023

Max: can you make the operator form before the transpilation? Seems like that'd save you both simulation cost, and solve your problem. Transpilation shouldn't affect the matrix representation (including global phase, although I know some of the higher-level synthesis stuff might be missing that right now), except for re-mapping some of the qubits, and that can be fixed on the Operator by applying permutations to the rows and columns for input and final layouts.

edit: I misunderstood what was going on a little bit - I was thinking needing matrix representations of the circuit and the operator, rather than the operator still being in a circuit or SparsePauliOp representation.

@mtreinish
Copy link
Member Author

mtreinish commented Jan 27, 2023

@mrossinek fwiw, this kind of thing is the intent of the Operator.from_circuit constructor method: https://qiskit.org/documentation/stubs/qiskit.quantum_info.Operator.from_circuit.html#qiskit.quantum_info.Operator.from_circuit which already factors in the initial layout and final, layout if it's present. Although, it currently does not factor in ancilla expansion reversal that you described, which is something is something @alexanderivrii brought up recently as well. We should open another issue to track that. Done in: #9476

But, yeah I think we all agree that the point here is that the _layout attribute has been semi-public for a long time because it's the only way users have to reason about the permutation caused by compilation. So we should promote it to be an actual public documented interface

@mtreinish
Copy link
Member Author

I'm also in favor of using a .layout attribute for the reasons @jakelishman outlined above along with two other reasons, for QPY metadata gets serialized by default by passing it to the default stdlib json serializer and deserializer. The interface we've defined for that is if users have anything outside what stdlib json can handle they should provide their own JSONEncoder and JSONDecoder subclasses to the qpy dump and load functions to be able to handle custom types. If we put the layout information in metadata we'd have to update the default serializers in qpy to be able to handle Layout and Qubit objects, but that would result in breaking any users that were using our interface as described, because if they expanded the metadata with their own custom types pre-transpilation then what previously worked for serializing a post-transpiled circuit would no longer work because their custom encoder and decoder would not handle the layout anymore unless they subclassed our custom encoder.

The other reason is having a separate attribute will let us clearly document the attribute. We can write a detailed description of the what the layout (as at this point it's really initial layout, input qubit mapping, and final layout) is and examples on how to use it. Having a place we can point people to to explain all of this I think will be quite valuable because explaining layout and routing as permutations can sometimes be confusing.

@nonhermitian
Copy link
Contributor

See my comment from May of last year that touches on the same issue:

Qiskit/qiskit-ibm-runtime#338

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jan 30, 2023
Since the Qiskit 0.9.0 release the QuantumCircuit has had a private
`_layout` attribute which is used to track the permutations introduced
to a circuit by the transpiler. At the time this was made private
because we didn't want to commit to an interface on this at the time
until a dedicated API could be introduced (see Qiskit#2853). However, since
that time ~4 years ago we've never introduced an alternative public
interface for `_layout` was never introduced. This has left users
needing to reason about the permutation caused by transpile() in a weird
place because the only mechanism to do this is private. This commit
adds a public read-only attribute `.layout` to solve this, which just
exposes the previously private `._layout` attribute via a documented and
public interface.

Unlike from 0.9.x through 0.21.x we introduced a new data class
container `TranspileLayout` as part of the 0.22.0 release which _layout
was updated to use at the time. Having a dedicated class used for the
field gives us a certain degree of flexibility to evolve the interface
over time if needed. Right now it contains 3 pieces of information the
initial layout caused by the layout phase of transpile(), the input
qubit mapping to figure out the qubit index on the original circuit, and
the final layout caused by swaps inserted during routing.

Closes Qiskit#8803
ElePT pushed a commit to ElePT/qiskit that referenced this issue Apr 5, 2023
* Add public layout attribute to QuantumCircuit

Since the Qiskit 0.9.0 release the QuantumCircuit has had a private
`_layout` attribute which is used to track the permutations introduced
to a circuit by the transpiler. At the time this was made private
because we didn't want to commit to an interface on this at the time
until a dedicated API could be introduced (see Qiskit#2853). However, since
that time ~4 years ago we've never introduced an alternative public
interface for `_layout` was never introduced. This has left users
needing to reason about the permutation caused by transpile() in a weird
place because the only mechanism to do this is private. This commit
adds a public read-only attribute `.layout` to solve this, which just
exposes the previously private `._layout` attribute via a documented and
public interface.

Unlike from 0.9.x through 0.21.x we introduced a new data class
container `TranspileLayout` as part of the 0.22.0 release which _layout
was updated to use at the time. Having a dedicated class used for the
field gives us a certain degree of flexibility to evolve the interface
over time if needed. Right now it contains 3 pieces of information the
initial layout caused by the layout phase of transpile(), the input
qubit mapping to figure out the qubit index on the original circuit, and
the final layout caused by swaps inserted during routing.

Closes Qiskit#8803

* Fix typo in QuantumCircuit.layout docs

* Disable cyclic import check on conditional type hint import
giacomoRanieri pushed a commit to giacomoRanieri/qiskit-terra that referenced this issue Apr 16, 2023
* Add public layout attribute to QuantumCircuit

Since the Qiskit 0.9.0 release the QuantumCircuit has had a private
`_layout` attribute which is used to track the permutations introduced
to a circuit by the transpiler. At the time this was made private
because we didn't want to commit to an interface on this at the time
until a dedicated API could be introduced (see Qiskit#2853). However, since
that time ~4 years ago we've never introduced an alternative public
interface for `_layout` was never introduced. This has left users
needing to reason about the permutation caused by transpile() in a weird
place because the only mechanism to do this is private. This commit
adds a public read-only attribute `.layout` to solve this, which just
exposes the previously private `._layout` attribute via a documented and
public interface.

Unlike from 0.9.x through 0.21.x we introduced a new data class
container `TranspileLayout` as part of the 0.22.0 release which _layout
was updated to use at the time. Having a dedicated class used for the
field gives us a certain degree of flexibility to evolve the interface
over time if needed. Right now it contains 3 pieces of information the
initial layout caused by the layout phase of transpile(), the input
qubit mapping to figure out the qubit index on the original circuit, and
the final layout caused by swaps inserted during routing.

Closes Qiskit#8803

* Fix typo in QuantumCircuit.layout docs

* Disable cyclic import check on conditional type hint import
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this issue May 22, 2023
* Add public layout attribute to QuantumCircuit

Since the Qiskit 0.9.0 release the QuantumCircuit has had a private
`_layout` attribute which is used to track the permutations introduced
to a circuit by the transpiler. At the time this was made private
because we didn't want to commit to an interface on this at the time
until a dedicated API could be introduced (see Qiskit#2853). However, since
that time ~4 years ago we've never introduced an alternative public
interface for `_layout` was never introduced. This has left users
needing to reason about the permutation caused by transpile() in a weird
place because the only mechanism to do this is private. This commit
adds a public read-only attribute `.layout` to solve this, which just
exposes the previously private `._layout` attribute via a documented and
public interface.

Unlike from 0.9.x through 0.21.x we introduced a new data class
container `TranspileLayout` as part of the 0.22.0 release which _layout
was updated to use at the time. Having a dedicated class used for the
field gives us a certain degree of flexibility to evolve the interface
over time if needed. Right now it contains 3 pieces of information the
initial layout caused by the layout phase of transpile(), the input
qubit mapping to figure out the qubit index on the original circuit, and
the final layout caused by swaps inserted during routing.

Closes Qiskit#8803

* Fix typo in QuantumCircuit.layout docs

* Disable cyclic import check on conditional type hint import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants