Port HighLevelSynthesis pass to rust #13813
Conversation
The interface is now quite ugly, and we also have an extra isinstance check for each gate, we will see if it can be removed.
This argument is not needed as the qubit tracker already restricts allowed ancilla qubits.
|
Thanks @raynelfss and @eliarbel (and transitively @jakelishman) for the feedback! I have applied all of Ray's and Eli's review suggestions, with the exception of modifying |
There was a problem hiding this comment.
I'm not quite through yet, but here's a first round of comments 🙂
Aside from the comments this looks like a good start to me. It would be great to set up a Trait for the Rust side of our plugins so that we can avoid going through Python to call them -- which I assume is what you already started on with #13929 😄
| Some(unitary) => unitary, | ||
| None => return Err(TranspilerError::new_err("Unitary not found")), | ||
| }; | ||
| match unitary.shape() { |
There was a problem hiding this comment.
Isn't this something that should be taken care of by unitary synthesis at the moment? Otherwise we're duplicating the logic and run the risk of hardcoding the target basis here, whereas the unitary synthesis can handle more general cases, no?
There was a problem hiding this comment.
I agree, I would have also liked to use some readily available unitary synthesis code :).
There was a problem hiding this comment.
As discussed with @Cryoris in private, in the future we want to synthesize unitary gates within HighLevelSynthesis using a plugin mechanism. For now, the current PR basically ports the definition method for unitary gates to Rust, without changing how they are handled.
|
Related to:
After a discussion with @Cryoris. Long-term it's better to have the code handling control-flow ops in rust (as done right now). There will be opportunities to simplify this code once other python attributes of the quantum circuit are fully ported to rust. Regarding:
Also after a discussion with @Cryoris. The "Trait" idea is really cool, and will allow to call the main synthesis methods for various high-level objects without going to the Python space. However, #13929 is orthogonal to this, it only aims to port more of the synthesis methods to rust. |
| # the operation is defined, and the initial state of each global qubit. | ||
| tracker = options.get("qubit_tracker", None) | ||
| data = options.get("hls_data", None) | ||
| input_qubits = options.get("input_qubits", None) |
There was a problem hiding this comment.
I've probably asked this before, but couldn't we use qubits here instead?
There was a problem hiding this comment.
Unfortunately, when the HighLevelSynthesis transpiler pass was first introduced (i.e. way before we started to support auxiliary qubits), we made the decision to pass qubits=None to plugins meaning that the pass runs pre-routing. But now that we handle recursion by explicitly reasoning about the qubits in the original circuit (aka the global qubits), we also need to pass the actual qubits as well. Ideally, I would like to have qubits always be the set of global qubits, and then we wouldn't need input_qubits, but this would be a breaking change. We do need to think of a path forward to implement this and other changes to the synthesis plugins API.
| circuit = circuit.inverse() | ||
|
|
||
| elif isinstance(modifier, ControlModifier): | ||
| if circuit.num_clbits > 0: |
There was a problem hiding this comment.
Let's revisit this in the future, but wouldn't the logic be exactly the same even with control flow logic? Since the quantum part would only be applied if the control are satisfied, the output should be correct, no?
There was a problem hiding this comment.
Right, so currently we are not allowing to control-annotate circuits with classical bits, and this PR does not change this behavior. I think you might be correct, and we should indeed revisit this in the future.
…n synthesizing the base operation of an annotated operation
There was a problem hiding this comment.
Modulo the failing test this LGTM, thanks for all the effort @alexanderivrii! There are some follow-ups we should not lose track of here, it might be good to have issues for them. The main ones I think are:
- Improve collaboration of
UnitarySynthesisandHighLevelSynthesis-- this needs some discussion to see if we want to combine them or, if not, how we can have HLS respect the unitary synthesis plugin selection - Use Rust-versions of HLS plugins from Rust directly (cf.
Traitconversation above) - Improve the control flow handling
I can approve once the tests are fixed 👍🏻
Summary
This PR ports [most of] the
HighLevelSynthesistranspiler pass (or HLS, for short) to Rust, addressing #12211.The
test_utility_scale.pyscript provides several examples where HLS does not need to do anything. Running this script on my laptop, reduces the total HLS runtime from about 25% of the total transpile time to just 0.3%.This PR is based on #13777 (a small fix for calling the
definitionmethods from within the Rust space) and #13605 (reimplementing HLS in Python). Technically, we can skip merging #13605 and merge PR directly, I don't have a strong preference for what is the better option.Details and comments
The part of the pass that still remains in Python is the code for synthesizing operations using plugins. As an optimization, we precompute the set of (names of) operations with available plugins and we only call this Python code when an operation is in this set. Note that annotated operations are also handled via a plugin.
The reason why porting the Python code that handles plugins is very tricky is because we allow passing arbitrary python classes in the
HLSConfig, something as follows:hls_config = HLSConfig("clifford"=[MyPythonCliffordSynthesisPlugin()]).In the case that the HighLevelSynthesis pass needs to do something (and in particular call synthesis plugins implemented in python), the runtime improvements are on the scale of 20%--50%.