Fix transpile() for control flow operations with a Target/BackendV2#8852
Conversation
This commit fixes an issue when compiling with circuits that have control flow operations and are running transpile() with a BackendV2 instance or a custom Target. In these cases the transpile() operation would fail to run because the Target didn't have a provision to recognize a global variable width operation as part of the target. Previously all operations in the target needed to have an instance of an Operation so that the parameters and number of qubits could be verified. Each of these operation instances would be assigned a unique name. But, for control flow operations they're defined over a variable number of qubits and all have the same name (this is a similar problem for gates like mcx too). Not being able to fully represent the control flow operations in a target was preventing running transpile() on a circuit with control flow. This commit fixes this by adding support to the target to represent globally defined operations by passing the class into Target.add_instruction instead of an instance of that class. When the class is received the Target class will treat that operation name and class as always being valid for the target for all qubits and parameters. This can then be used to represent control flow operations in the target and run transpile() with control flow operations. Fixes Qiskit#8824
|
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:
|
mtreinish
added
the
stable backport potential
|
I believe the test failures here are being caused by a bug in |
|
The bug looks like it must be in |
In Qiskit/qiskit#8852 support is being added to the Target class for representing globally available variable width operations in order to support control flow operations. This will be included in the 0.22.0 release. When IBM backends start listing that they support control flow operations we need to be constructing the Target for the backend with the control flow operations. This commit adds the necessary handling so that when a backend lists the control flow operations in the configuration payload we add the instructions correctly to the Target.
mtreinish
changed the title
jakelishman
left a comment
jakelishman
left a comment
There was a problem hiding this comment.
This mostly looks good to me, thanks. I had a few questions, but in general the new interface seems sensible to me.
It technically breaks the public API for consumers of the Target class (since the output type signature of Target.instructions and a couple of others change), but I think that might be a necessary evil.
I think for |
jakelishman
left a comment
jakelishman
left a comment
There was a problem hiding this comment.
Looks good, thanks.
What I meant by the API break was that something that was processing existing Targets could be broken by this - more things than before are now valid Target instances, so someone could pass a new Target (say the new ibm-provider ones) into a method that processes targets, and the method could fail through no fault of its own. I really don't think that's actually happening anywhere in practice, though.
…8852) * Fix transpile() for control flow operations with a Target/BackendV2 This commit fixes an issue when compiling with circuits that have control flow operations and are running transpile() with a BackendV2 instance or a custom Target. In these cases the transpile() operation would fail to run because the Target didn't have a provision to recognize a global variable width operation as part of the target. Previously all operations in the target needed to have an instance of an Operation so that the parameters and number of qubits could be verified. Each of these operation instances would be assigned a unique name. But, for control flow operations they're defined over a variable number of qubits and all have the same name (this is a similar problem for gates like mcx too). Not being able to fully represent the control flow operations in a target was preventing running transpile() on a circuit with control flow. This commit fixes this by adding support to the target to represent globally defined operations by passing the class into Target.add_instruction instead of an instance of that class. When the class is received the Target class will treat that operation name and class as always being valid for the target for all qubits and parameters. This can then be used to represent control flow operations in the target and run transpile() with control flow operations. Fixes #8824 * Simplify test slightly * Add release note * Add coupling map target test * Raise if instruction class and properties are both set * Only return global gates if they exist * Change UserWarning to a comment * Revert change to instructions() getter * Add release note about coupling map api change * Fix lint and logic bug * Add back nested while_loop to transpile() test (cherry picked from commit ad07847)
…8852) (#8889) * Fix transpile() for control flow operations with a Target/BackendV2 This commit fixes an issue when compiling with circuits that have control flow operations and are running transpile() with a BackendV2 instance or a custom Target. In these cases the transpile() operation would fail to run because the Target didn't have a provision to recognize a global variable width operation as part of the target. Previously all operations in the target needed to have an instance of an Operation so that the parameters and number of qubits could be verified. Each of these operation instances would be assigned a unique name. But, for control flow operations they're defined over a variable number of qubits and all have the same name (this is a similar problem for gates like mcx too). Not being able to fully represent the control flow operations in a target was preventing running transpile() on a circuit with control flow. This commit fixes this by adding support to the target to represent globally defined operations by passing the class into Target.add_instruction instead of an instance of that class. When the class is received the Target class will treat that operation name and class as always being valid for the target for all qubits and parameters. This can then be used to represent control flow operations in the target and run transpile() with control flow operations. Fixes #8824 * Simplify test slightly * Add release note * Add coupling map target test * Raise if instruction class and properties are both set * Only return global gates if they exist * Change UserWarning to a comment * Revert change to instructions() getter * Add release note about coupling map api change * Fix lint and logic bug * Add back nested while_loop to transpile() test (cherry picked from commit ad07847) Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
In Qiskit#8852 we added support to the target class for representing variable width operations by using the class directly instead of an instance. Now that we have settled on a representation for these types of operations we can add them to the built in name mapping function (from which they were previously excluded. This commit adds the built-in variable width operations to the output of the function and returns just the class instead of an instance.
* Add support for building Targets with control flow operations In Qiskit/qiskit#8852 support is being added to the Target class for representing globally available variable width operations in order to support control flow operations. This will be included in the 0.22.0 release. When IBM backends start listing that they support control flow operations we need to be constructing the Target for the backend with the control flow operations. This commit adds the necessary handling so that when a backend lists the control flow operations in the configuration payload we add the instructions correctly to the Target. * Bump qiskit-terra requirement to support change Co-authored-by: Kevin Tian <kevin.tian@ibm.com>
3 participants
Summary
This commit fixes an issue when compiling with circuits that have control flow operations and are running transpile() with a BackendV2 instance or a custom Target. In these cases the transpile() operation would fail to run because the Target didn't have a provision to recognize a global variable width operation as part of the target. Previously all operations in the target needed to have an instance of an Operation so that the parameters and number of qubits could be verified. Each of these operation instances would be assigned a unique name. But, for control flow operations they're defined over a variable number of qubits and all have the same name (this is a similar problem for gates like mcx too). Not being able to fully represent the control flow operations in a target was preventing running transpile() on a circuit with control flow. This commit fixes this by adding support to the target to represent globally defined operations by passing the class into Target.add_instruction instead of an instance of that class. When the class is received the Target class will treat that operation name and class as always being valid for the target for all qubits and parameters. This can then be used to represent control flow operations in the target and run transpile() with control flow operations.
Details and comments
Fixes #8824
TODO: