Make `BasisTranslator` rust-native. by raynelfss · Pull Request #14659 · Qiskit/qiskit

raynelfss · 2025-06-23T15:44:45Z

Summary

The following commits expose a more rust native BasisTranslator that operates mostly on the Rust level by taking the following into account:

Extracting attributes from the Target in Rust-space instead of Python.
Utilizing the rust-native ParameterExpression to correctly handle parameters.
Separate the layers of functionality that are meant to only work with Python.
Using Rust native errors of the type BasisTranslatorError to correclty represent errors without having to go through Python.

Details and comments

Serves as the prelude for #14874.
~~This PR is currently built on top of #14799 and needs to be rebased once it merges. Rebased now with #14766~~ REBASED!

Pre-requisites:

coveralls · 2025-07-07T20:46:09Z

Pull Request Test Coverage Report for Build 17215159484

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

344 of 469 (73.35%) changed or added relevant lines in 5 files are covered.
30 unchanged lines in 8 files lost coverage.
Overall coverage decreased (-0.07%) to 88.382%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/circuit/src/parameter/parameter_expression.rs	0	3	0.0%
crates/transpiler/src/passes/basis_translator/errors.rs	4	7	57.14%
crates/transpiler/src/passes/basis_translator/compose_transforms.rs	87	110	79.09%
crates/transpiler/src/passes/basis_translator/mod.rs	251	347	72.33%

Files with Coverage Reduction	New Missed Lines	%
crates/circuit/src/dag_circuit.rs	1	85.28%
crates/qasm2/src/expr.rs	1	93.63%
qiskit/circuit/library/hamiltonian_gate.py	1	82.35%
crates/qasm2/src/lex.rs	4	92.27%
crates/circuit/src/operations.rs	5	85.35%
crates/circuit/src/parameter/parameter_expression.rs	5	82.81%
qiskit/circuit/library/pauli_evolution.py	5	95.1%
crates/circuit/src/parameter/symbol_expr.rs	8	72.82%

Totals
Change from base Build 17203225128:	-0.07%
Covered Lines:	89899
Relevant Lines:	101716

💛 - Coveralls

raynelfss · 2025-08-05T03:59:17Z

There are some significant speedups:

Change	Before [`4810458`] <stash~10^2>	After [`bb10ab7`]	Ratio	Benchmark (Parameter)
-	47.7±0.4ms	39.1±0.5ms	0.82	passes.MultipleBasisPassBenchmarks.time_basis_translator(14, 1024, ['rx', 'ry', 'rz', 'r', 'rxx', 'id'])
-	20.4±0.4ms	15.4±0.2ms	0.75	passes.MultipleBasisPassBenchmarks.time_basis_translator(5, 1024, ['rx', 'ry', 'rz', 'r', 'rxx', 'id'])
-	57.2±2ms	33.8±0.2ms	0.59	passes.MultipleBasisPassBenchmarks.time_basis_translator(20, 1024, ['rz', 'x', 'sx', 'cx', 'id'])
-	51.8±1ms	25.5±0.5ms	0.49	passes.MultipleBasisPassBenchmarks.time_basis_translator(20, 1024, ['u', 'cx', 'id'])
-	18.0±0.2ms	8.47±0.08ms	0.47	passes.MultipleBasisPassBenchmarks.time_basis_translator(5, 1024, ['rz', 'x', 'sx', 'cx', 'id'])
-	37.8±0.2ms	17.4±0.2ms	0.46	passes.MultipleBasisPassBenchmarks.time_basis_translator(14, 1024, ['u', 'cx', 'id'])
-	14.7±0.4ms	6.50±0.1ms	0.44	passes.MultipleBasisPassBenchmarks.time_basis_translator(5, 1024, ['u', 'cx', 'id'])
-	43.8±1ms	17.7±0.3ms	0.4	passes.MultipleBasisPassBenchmarks.time_basis_translator(14, 1024, ['rz', 'x', 'sx', 'cx', 'id'])

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

qiskit-bot · 2025-08-12T20:33:32Z

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

@Qiskit/terra-core

…ive.

- We cannot get away with not using `ParameterExpression` to create unique parameters. Until a better alternative comes around, we should stick to using it.

- As of 1.79, `LazyLock` was not yet made stable, these commits remove `LazyLock` in favor of `OnceLock`.

- Skipping the dict builds and using `Python::with_gil` avoids using python tokens when not necessary during the `replace_node` method in `run_basis_translator`. - The dictionary in question was only really needed when an instance of `ParameterExpression` is present on the gate. So it is optionally built if an instance of Parameter expression is found. - The last remaining usage of py tokens exists during `compose_transforms` which still builds a parameter vector.

- In `replace_node` we will first filter out `ParameterExpression` keys in the parameter map. If the collection comes out empty, we don't build the mapping.

Cryoris · 2025-08-25T09:03:02Z

+    for key in param_obj.iter_symbols() {
+        match &parameter_map[key].clone() {
+            Param::ParameterExpression(val) => {
+                subs_map.insert(key.clone(), val.as_ref().clone());


Oh we should update subs to also take &Symbol as key in a separate PR

Cryoris · 2025-08-25T09:36:26Z

+}

-            _ => {}
+fn param_assignment_expr(


Could we make this a method on Param? We could also do that in a follow-up if you prefer.

@Cryoris

As per @Cryoris previous reviews, there is no path for a `UnitaryGate` to be represented as a key in the `EquivalenceLibrary`. Thus we can remove the unitary path from the `name_to_packed_operation` method.

- Fix comment in `recurse_circuit`. - Remove `param_assignment_global_phase` and use `param_assignment_expr. - `param_assignment_expr` now accepts a `ParameterExpression` instance. - Add `allow_complex` flag in `param_assignment_expr` to prevent complex values from getting assigned.

Cryoris · 2025-08-25T15:23:20Z

-            .get_bound(py)
-            .call1((&gate_name, num_params))?
-            .extract()?;
+        let mut placeholder_params: SmallVec<[Param; 3]> = (0..num_params as u32)


Do these need to be mut? I don't think we change them, do we?

We change them when a python gate is made and we re-extract the parameters from there. I can have it removed though.

Ah right, no then they are indeed mut

Cryoris · 2025-08-25T15:25:58Z

+        let inst = match name {
+            "barrier" => StandardInstruction::Barrier(num_qubits),
+            "delay" => StandardInstruction::Delay(qiskit_circuit::operations::DelayUnit::DT),
+            "measure" => StandardInstruction::Measure,
+            "reset" => StandardInstruction::Reset,


I don't think we contain translations for instructions, do we? I think we can also replace this whole block with an unreachable statement like for the unitary

Sure! Couldn't remove this unfortunately. The specified target basis doesn't filter out the StandardInstructions.

Cryoris · 2025-08-25T15:39:24Z

+    equiv_lib: &mut EquivalenceLibrary,
+    min_qubits: usize,
+    target: Option<&Target>,
+    target_basis: Option<HashSet<String>>,


We should take an Option<&HashSet<&str>> in the Rust interface I think, and create the correct type in the python wrapper above 🤔

In general I agree we don't need or want an owned type here, but in practice we should never be setting this from Rust, it should always be None. In rust space we should always be using a Target and never loose constraints, this is strictly a backwards compatibility thing for Python.

It would be better if we just removed this argument from the rust code and did Target.from_configuration(basis_gates) in the Python code for the pass instead and pass the generated target to rust. That would also simplify the rust code so we only have a target to worry about.

Added in 523a30b

- Remove hashset from `compose_transforms` use array too. - Fix comment in error. - Build `HashSet<&str>` from python counterpart and use as main `target_basis` argument in `run_basis_translator`.

Cryoris

Thanks for all the work, this looks in a good shape to me now to unblock the C transpiler function. There's some follow-ups which we can do but with the beta release in mind I would be happy to merge it 🙂

The main follow ups are:

#14659 (comment) (this we could still do here if you want, but I'm fine doing a follow up to unblock now)
the Rust-native errors
putting parameter assignments as methods on Param and ParameterExpression instead of standalones

@Cryoris

* TBD * TBD 0 * TBD: Moved most of the operations in compose_transform to be rust native. * Fix: Revert any removals related to `ParameterExpression`. - We cannot get away with not using `ParameterExpression` to create unique parameters. Until a better alternative comes around, we should stick to using it. * Fix: Remove `LazyLock`. - As of 1.79, `LazyLock` was not yet made stable, these commits remove `LazyLock` in favor of `OnceLock`. * Fix: Remove most pytokens by avoiding building a dict. - Skipping the dict builds and using `Python::with_gil` avoids using python tokens when not necessary during the `replace_node` method in `run_basis_translator`. - The dictionary in question was only really needed when an instance of `ParameterExpression` is present on the gate. So it is optionally built if an instance of Parameter expression is found. - The last remaining usage of py tokens exists during `compose_transforms` which still builds a parameter vector. * Filter to only parameter expression when building parameter_map. - In `replace_node` we will first filter out `ParameterExpression` keys in the parameter map. If the collection comes out empty, we don't build the mapping. * Fix: Use rust native parameters. * Fix: Re-add "fixedbitset" * Fix: Convert to `Param::Float` if a real value is obtained * Fix: Second rebase * Add: Leverage use of rust-native `substitute_node_with_py`. * Fix: Implement native errors for `BasisTranslator`. - Use `Matrix::Identity` based on the provided matrices. - Add trait `ToPyErr` that ensures any new error is converted to its respective Python counterpart. - Remove indexing from `ParameterExpression` creation in `compose_transforms`. - Remove incorrect coercion into value from `replace_node`. * Add: global operation extraction from Rust. - Modify `Target::get_non_global_operation_names` to not return an option. * Fix: Remove unused import in python `BasisTranslator`. * Refactor: `ToPyError` to `From<BasisTranslatorError> for PyErr`. * Chore: Rename name set in `compose_transforms`. - Use acquired gil for `to_string() call of `PyErr`. * Fix: Take `&mut DAGCircuit` in `BasisTranslator`. * Fix: Address rest of review comments Co-authored-by: Julien Gacon <gaconju@gmail.com> * Refactor: Use `&str` over `String` when possible. * Chore: Round of review comments. * Fix: More review comments - Use rust native dag_to_circuit method and convert to `QuantumCircuit` for control flow procedures. - Add `num_symbols` to `ParameterExpression`. - Pass `allow_unknwon_parameters` as true for `bind` and `subs` in `BasisTranslator`. Co-authored-by: Julien Gacon <gaconju@gmail.com> * Fix: Split out parameter assignment from `replace_node` * Fix: More efficient parameter binding Co-authored-by: Julien Gacon <gaconju@gmail.com> * Remove: Path for unitaries. As per @Cryoris previous reviews, there is no path for a `UnitaryGate` to be represented as a key in the `EquivalenceLibrary`. Thus we can remove the unitary path from the `name_to_packed_operation` method. * Fix: Remove most string conversions from `BasisTranslatorError`. * Fix: Address first wave of review items - Fix comment in `recurse_circuit`. - Remove `param_assignment_global_phase` and use `param_assignment_expr. - `param_assignment_expr` now accepts a `ParameterExpression` instance. - Add `allow_complex` flag in `param_assignment_expr` to prevent complex values from getting assigned. * Refactor: Use `ParameterError` as a base for `BasisTranslatorError`. * FIx: Remove `BasisGateError`, use `.expect()` for the rest. * Fix: Final review comments? - Remove hashset from `compose_transforms` use array too. - Fix comment in error. - Build `HashSet<&str>` from python counterpart and use as main `target_basis` argument in `run_basis_translator`. * Fix: Re-add removed parameter extraction --------- Co-authored-by: Julien Gacon <gaconju@gmail.com>

@Cryoris

* TBD * TBD 0 * TBD: Moved most of the operations in compose_transform to be rust native. * Fix: Revert any removals related to `ParameterExpression`. - We cannot get away with not using `ParameterExpression` to create unique parameters. Until a better alternative comes around, we should stick to using it. * Fix: Remove `LazyLock`. - As of 1.79, `LazyLock` was not yet made stable, these commits remove `LazyLock` in favor of `OnceLock`. * Fix: Remove most pytokens by avoiding building a dict. - Skipping the dict builds and using `Python::with_gil` avoids using python tokens when not necessary during the `replace_node` method in `run_basis_translator`. - The dictionary in question was only really needed when an instance of `ParameterExpression` is present on the gate. So it is optionally built if an instance of Parameter expression is found. - The last remaining usage of py tokens exists during `compose_transforms` which still builds a parameter vector. * Filter to only parameter expression when building parameter_map. - In `replace_node` we will first filter out `ParameterExpression` keys in the parameter map. If the collection comes out empty, we don't build the mapping. * Fix: Use rust native parameters. * Fix: Re-add "fixedbitset" * Fix: Convert to `Param::Float` if a real value is obtained * Fix: Second rebase * Add: Leverage use of rust-native `substitute_node_with_py`. * Fix: Implement native errors for `BasisTranslator`. - Use `Matrix::Identity` based on the provided matrices. - Add trait `ToPyErr` that ensures any new error is converted to its respective Python counterpart. - Remove indexing from `ParameterExpression` creation in `compose_transforms`. - Remove incorrect coercion into value from `replace_node`. * Add: global operation extraction from Rust. - Modify `Target::get_non_global_operation_names` to not return an option. * Fix: Remove unused import in python `BasisTranslator`. * Refactor: `ToPyError` to `From<BasisTranslatorError> for PyErr`. * Chore: Rename name set in `compose_transforms`. - Use acquired gil for `to_string() call of `PyErr`. * Fix: Take `&mut DAGCircuit` in `BasisTranslator`. * Fix: Address rest of review comments Co-authored-by: Julien Gacon <gaconju@gmail.com> * Refactor: Use `&str` over `String` when possible. * Chore: Round of review comments. * Fix: More review comments - Use rust native dag_to_circuit method and convert to `QuantumCircuit` for control flow procedures. - Add `num_symbols` to `ParameterExpression`. - Pass `allow_unknwon_parameters` as true for `bind` and `subs` in `BasisTranslator`. Co-authored-by: Julien Gacon <gaconju@gmail.com> * Fix: Split out parameter assignment from `replace_node` * Fix: More efficient parameter binding Co-authored-by: Julien Gacon <gaconju@gmail.com> * Remove: Path for unitaries. As per @Cryoris previous reviews, there is no path for a `UnitaryGate` to be represented as a key in the `EquivalenceLibrary`. Thus we can remove the unitary path from the `name_to_packed_operation` method. * Fix: Remove most string conversions from `BasisTranslatorError`. * Fix: Address first wave of review items - Fix comment in `recurse_circuit`. - Remove `param_assignment_global_phase` and use `param_assignment_expr. - `param_assignment_expr` now accepts a `ParameterExpression` instance. - Add `allow_complex` flag in `param_assignment_expr` to prevent complex values from getting assigned. * Refactor: Use `ParameterError` as a base for `BasisTranslatorError`. * FIx: Remove `BasisGateError`, use `.expect()` for the rest. * Fix: Final review comments? - Remove hashset from `compose_transforms` use array too. - Fix comment in error. - Build `HashSet<&str>` from python counterpart and use as main `target_basis` argument in `run_basis_translator`. * Fix: Re-add removed parameter extraction --------- Co-authored-by: Julien Gacon <gaconju@gmail.com>

…slator`. - A well known bug introduced by Qiskit#14659 in which the `AerSimulator` is known to define `UnitaryGate` as part of its target basis. However, `BasisTranslator` should not include `UnitaryGate` as part of its calculations, it should instead be handled by `UnitarySynthesis`. - To fix this, `BasisTranslator` skips any `UnitaryGate` in the circuit by filtering its `op_nodes` to check for any `UnitaryGate` instances. - The only question left is whether we should still have the panic condition in `name_to_packed_operation` because this check is done on a name basis rather than instance.

raynelfss force-pushed the basis-leverage-rust-n branch 4 times, most recently from c53b7ec to 6ccba5a Compare August 4, 2025 13:33

raynelfss changed the title ~~[WIP] Basis leverage rust native~~ [WIP] Make BasisTranslator rust-native. Aug 4, 2025

raynelfss added Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository mod: transpiler Issues and PRs related to Transpiler labels Aug 4, 2025

raynelfss added this to the 2.2.0 milestone Aug 4, 2025

raynelfss force-pushed the basis-leverage-rust-n branch 3 times, most recently from 0526837 to d46bcd2 Compare August 11, 2025 14:53

This was referenced Aug 12, 2025

C API: Burn BasisTranslator #14874

Merged

Add BasisTranslator function to the C API #14441

Closed

raynelfss force-pushed the basis-leverage-rust-n branch from b978929 to 9081202 Compare August 12, 2025 20:32

raynelfss marked this pull request as ready for review August 12, 2025 20:33

raynelfss requested a review from a team as a code owner August 12, 2025 20:33

raynelfss changed the title ~~[WIP] Make BasisTranslator rust-native.~~ Make BasisTranslator rust-native. Aug 12, 2025

raynelfss and others added 9 commits August 13, 2025 13:42

TBD

addab09

TBD 0

6a10c43

TBD: Moved most of the operations in compose_transform to be rust nat…

5e634e9

…ive.

Fix: Revert any removals related to ParameterExpression.

d73fcd2

- We cannot get away with not using `ParameterExpression` to create unique parameters. Until a better alternative comes around, we should stick to using it.

Fix: Remove LazyLock.

d389018

- As of 1.79, `LazyLock` was not yet made stable, these commits remove `LazyLock` in favor of `OnceLock`.

Filter to only parameter expression when building parameter_map.

3e98595

- In `replace_node` we will first filter out `ParameterExpression` keys in the parameter map. If the collection comes out empty, we don't build the mapping.

Fix: Use rust native parameters.

30cc3de

Fix: Re-add "fixedbitset"

8edab5d

raynelfss force-pushed the basis-leverage-rust-n branch from 49e7207 to 9d3aad6 Compare August 22, 2025 17:32

Merge remote-tracking branch 'upstream/main' into basis-leverage-rust-n

d0cedb3