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

Port HighLevelSynthesis pass to rust #13813

Open
wants to merge 71 commits into
base: main
Choose a base branch
from

Conversation

alexanderivrii
Copy link
Contributor

@alexanderivrii alexanderivrii commented Feb 9, 2025

Summary

This PR ports [most of] the HighLevelSynthesis transpiler pass (or HLS, for short) to Rust, addressing #12211.

The test_utility_scale.py script 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 definition methods 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%.

@alexanderivrii alexanderivrii added this to the 2.0.0 milestone Feb 9, 2025
@alexanderivrii alexanderivrii requested a review from a team as a code owner February 9, 2025 15:00
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

Even when HighLevelSynthesis runs directly after UnitarySynthesis, it may happen that
certain definitions involve UnitaryGates (for instance, this is the case for Isometry),
in which case the default behavior of HighLevelSynthesis should be to query the
definition of UnitaryGates if they are not in the basis.
Going from QuantumCircuit/DAGCircuit to CircuitData discards information,
such as information about registers, input variables and more. The previously
tried approach of constructing DAGCircuit/QuantumCircuitData in rust space by
calling _from_circuit_data and manually fixing registers was not complete.
Instead we now call clone_empty_like both on DAGCircuit and QuantumCircuit.
and avoiding expensive cloning
this actually requires dill since HLSConfig includes a lambda function
for comparing circuits obtained with different synthesis plugins,
but fortunately it's all already supported
This avoids cloning data when calling Python
This avoid having to clone it when calling the Python space
@coveralls
Copy link

coveralls commented Feb 13, 2025

Pull Request Test Coverage Report for Build 13493411156

Details

  • 638 of 679 (93.96%) changed or added relevant lines in 7 files are covered.
  • 37 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+0.03%) to 88.148%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/synthesis/plugin.py 1 2 50.0%
qiskit/transpiler/passes/synthesis/high_level_synthesis.py 62 64 96.88%
qiskit/transpiler/passes/synthesis/hls_plugins.py 57 66 86.36%
crates/accelerate/src/high_level_synthesis.rs 495 524 94.47%
Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/passes/synthesis/plugin.py 1 84.34%
crates/circuit/src/dag_circuit.rs 3 87.54%
crates/qasm2/src/lex.rs 3 93.23%
crates/circuit/src/circuit_instruction.rs 4 79.14%
crates/accelerate/src/high_level_synthesis.rs 5 91.51%
crates/circuit/src/dag_node.rs 9 74.86%
crates/qasm2/src/parse.rs 12 96.68%
Totals Coverage Status
Change from base Build 13469145802: 0.03%
Covered Lines: 78778
Relevant Lines: 89370

💛 - Coveralls

The main functionality is to create some default definition for unitary gates, for all other
operation types we can simply use the definition method.
With various conversions of DAGCircuit to QuantumCircuitData to CircuitData in Rust,
it's best to make sure that the global phases appearing in circuits/control-flow subcircuits
are tracked correctly
@1ucian0 1ucian0 linked an issue Feb 13, 2025 that may be closed by this pull request
Comment on lines +1138 to +1160
/// Append a packed operation to this CircuitData
pub fn push_packed_operation(
&mut self,
operation: PackedOperation,
params: &[Param],
qargs: &[Qubit],
cargs: &[Clbit],
) -> PyResult<()> {
let params = (!params.is_empty()).then(|| Box::new(params.iter().cloned().collect()));
let qubits = self.qargs_interner.insert(qargs);
let clbits = self.cargs_interner.insert(cargs);
self.data.push(PackedInstruction {
op: operation,
qubits,
clbits,
params,
extra_attrs: ExtraInstructionAttributes::default(),
#[cfg(feature = "cache_pygates")]
py_op: OnceLock::new(),
});
Ok(())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed this function in addition to the already existing function push_standard_gate, but the code is literally identical. I am wondering if these two versions can be somehow combined?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the difference is that push_standard_gate is optimized a bit for gates. We know that a gate has no clbits so we don't bother calling cargs_interner.insert()`. The idea was to save the overhead of having to repeatedly query the interner when we know ahead of time that it's going to be the same default value for an empty list.

There is probably a path we could add to unify the functions, maybe change the method internals to match based on the type of operation so we avoid the hash lookup if it's a StandardGate, PyGate, or UnitaryGate. But maybe look into that in a separate PR to avoid too much complexity here since this is already pretty large.

@alexanderivrii alexanderivrii changed the title [WIP] Port HighLevelSynthesis pass to rust Port HighLevelSynthesis pass to rust Feb 13, 2025
@mtreinish mtreinish added performance Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: transpiler Issues and PRs related to Transpiler performance Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port HighLevelSynthesis to rust
5 participants