Port Unroll3qOrMore to Rust #14835
Port Unroll3qOrMore to Rust #14835
Conversation
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 16920565451Details
💛 - Coveralls |
b5b94bd to
107b1af
Compare
107b1af to
c37280e
Compare
|
With #14766 merging this is now unblocked and ready to review. |
This commit ports the implementation of the Unroll3qOrMore transpiler pass to Rust. Functionally in Python we don't use this pass by default anymore by default because it's been superseded by HighLevelSynthesis which does unrolling of custom operations for us. However, in the C transpilation API we don't expose high level synthesis because we don't expose custom operations to C yet. So to handle multiqubit gates in the circuit we need the unroll 3q or more pass to serve the purpose it did before high level synthesis. The first step for that is porting the implementation of the pass to rust which will then let us call it from C without Python. Fixes Qiskit#12247
| #[error("Failed to substitute the definition")] | ||
| SubstitutionError(PyErr), |
There was a problem hiding this comment.
I may not have fully understood the discussion around using PyErr for the C API, is it fine to keep the PyErr here?
There was a problem hiding this comment.
The issue with PyErr for the C API is that we can't get any an error message or anything to return to see from it. It is a pure rust error type, but it's opaque without the GIL. It's fine to return it as long as we don't need to introspect the error for any details. In this case I put an error message on the error type here which we can use for returning to the user in C if needed ("Failed to substitute the definition"), although I'm not sure we'll need to return it to C, it's not really recoverable. This will let us keep the the original PyErr opaque and untouched in a C context. But for a Python context we can return the python space error returned.
More generally the use of PyErr is in a lot of the C API right now and will need to be cleaned up in a larger refactor.
There was a problem hiding this comment.
Thanks for the explanation.
| if target is None: | ||
| self.target = Target.from_configuration( | ||
| [x for x in basis_gates if x not in CONTROL_FLOW_OP_NAMES] | ||
| ) |
There was a problem hiding this comment.
This is nice, and explains why the Rust version of the pass does not need basis gates.
This commit ports the implementation of the Unroll3qOrMore transpiler pass to Rust. Functionally in Python we don't use this pass by default anymore by default because it's been superseded by HighLevelSynthesis which does unrolling of custom operations for us. However, in the C transpilation API we don't expose high level synthesis because we don't expose custom operations to C yet. So to handle multiqubit gates in the circuit we need the unroll 3q or more pass to serve the purpose it did before high level synthesis. The first step for that is porting the implementation of the pass to rust which will then let us call it from C without Python. Fixes Qiskit#12247
Summary
This commit ports the implementation of the Unroll3qOrMore transpiler
pass to Rust. Functionally in Python we don't use this pass by default
anymore by default because it's been superseded by HighLevelSynthesis
which does unrolling of custom operations for us. However, in the C
transpilation API we don't expose high level synthesis because we don't
expose custom operations to C yet. So to handle multiqubit gates in the
circuit we need the unroll 3q or more pass to serve the purpose it did
before high level synthesis. The first step for that is porting the
implementation of the pass to rust which will then let us call it from C
without Python.
Details and comments
Fixes #12247
This PR is based on #14766 and it will need to be rebased after #14766 merges this will need to be rebased. In the meantime you can view the contents of just this PR by looking at the HEAD commit: b5b94bdThis has been rebased now.