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 BasisTranslator to Rust #12246

Closed
4 tasks done
Tracked by #12208
mtreinish opened this issue Apr 19, 2024 · 0 comments · Fixed by #13237
Closed
4 tasks done
Tracked by #12208

Port BasisTranslator to Rust #12246

mtreinish opened this issue Apr 19, 2024 · 0 comments · Fixed by #13237
Assignees
Labels
mod: transpiler Issues and PRs related to Transpiler performance priority: high Rust This PR or issue is related to Rust code in the repository
Milestone

Comments

@mtreinish
Copy link
Member

mtreinish commented Apr 19, 2024

This is blocked by #11721, #12205, and #12278.

Parts of the BasisTranslator

Preview Give feedback
  1. Changelog: None Rust mod: circuit performance priority: high
  2. Changelog: None Rust performance
  3. Changelog: None Rust performance
  4. Changelog: None Rust performance priority: high
    ElePT
@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 Apr 19, 2024
@raynelfss raynelfss self-assigned this Jun 17, 2024
@mtreinish mtreinish added this to the 1.3 beta milestone Aug 15, 2024
@mtreinish mtreinish modified the milestones: 1.3 beta, 1.3.0 Sep 6, 2024
raynelfss added a commit to raynelfss/qiskit that referenced this issue Sep 27, 2024
Fixes Port `BasisTranslator` to Rust Qiskit#12246

This is the final act of the efforts to move the `BasisTranslator` transpiler pass into Rust. With many of the parts of this pass already living in Rust, the following commits attempt to bring in the final changes to allow complete operation of this pass in the Rust space (with of course some interaction with Python.)

Methodology:
The way this works is by keeping the original `BasisTranslator` python class, and have it store the rust-space counterpart (now called `CoreBasisTranslator`) which will perform all of the operations leveraging the existent Rust API's available for the `Target` (Qiskit#12292), `EquivalenceLibrary`(Qiskit#12585) and the `BasisTranslator` methods `basis_search` (Qiskit#12811) and `compose_transforms`(Qiskit#13137).

All of the inner methods will have private visibility and will not be accessible to `Python` as they're intended to be internal by design.

By removing the extra layers of conversion we should be seeing a considerable speed-up, alongside all of the other incremental improvements we have made.

Changes:

- Add the pyo3 class/struct `BasisTranslator` that will contain allof the main data used by the transpiler pass to perform its operation.
- Convert the `target_basis` into a set manually from python before sending it into the Rust space.
- Remove the exposure of `basis_search` and `compose_transforms` to python.
- Change `basis_search` so that it accepts references to `HashSet` instances instead of accepting a `HashSet<&str>` instance.
- Change inner method's visibility for `basis_search` and `compose_transform` modules in rust.
- Expose the exception imports from `Target` to the `accelerate` crate.
- Expose `DAGCircuit::copy_empty_like` to the rest of the crates.
- Remove all of the unused imports in the Python-side `BasisTranslator`.

Blockers:
- [ ] Qiskit#12811
raynelfss added a commit to raynelfss/qiskit that referenced this issue Oct 8, 2024
Fixes Port `BasisTranslator` to Rust Qiskit#12246

This is the final act of the efforts to move the `BasisTranslator` transpiler pass into Rust. With many of the parts of this pass already living in Rust, the following commits attempt to bring in the final changes to allow complete operation of this pass in the Rust space (with of course some interaction with Python.)

Methodology:
The way this works is by keeping the original `BasisTranslator` python class, and have it store the rust-space counterpart (now called `CoreBasisTranslator`) which will perform all of the operations leveraging the existent Rust API's available for the `Target` (Qiskit#12292), `EquivalenceLibrary`(Qiskit#12585) and the `BasisTranslator` methods `basis_search` (Qiskit#12811) and `compose_transforms`(Qiskit#13137).

All of the inner methods will have private visibility and will not be accessible to `Python` as they're intended to be internal by design.

By removing the extra layers of conversion we should be seeing a considerable speed-up, alongside all of the other incremental improvements we have made.

Changes:

- Add the pyo3 class/struct `BasisTranslator` that will contain allof the main data used by the transpiler pass to perform its operation.
- Convert the `target_basis` into a set manually from python before sending it into the Rust space.
- Remove the exposure of `basis_search` and `compose_transforms` to python.
- Change `basis_search` so that it accepts references to `HashSet` instances instead of accepting a `HashSet<&str>` instance.
- Change inner method's visibility for `basis_search` and `compose_transform` modules in rust.
- Expose the exception imports from `Target` to the `accelerate` crate.
- Expose `DAGCircuit::copy_empty_like` to the rest of the crates.
- Remove all of the unused imports in the Python-side `BasisTranslator`.

Blockers:
- [ ] Qiskit#12811
github-merge-queue bot pushed a commit that referenced this issue Oct 31, 2024
…Rust. (#13237)

* Initial: Move the rest of the `BasisTranslator` to Rust.

Fixes Port `BasisTranslator` to Rust #12246

This is the final act of the efforts to move the `BasisTranslator` transpiler pass into Rust. With many of the parts of this pass already living in Rust, the following commits attempt to bring in the final changes to allow complete operation of this pass in the Rust space (with of course some interaction with Python.)

Methodology:
The way this works is by keeping the original `BasisTranslator` python class, and have it store the rust-space counterpart (now called `CoreBasisTranslator`) which will perform all of the operations leveraging the existent Rust API's available for the `Target` (#12292), `EquivalenceLibrary`(#12585) and the `BasisTranslator` methods `basis_search` (#12811) and `compose_transforms`(#13137).

All of the inner methods will have private visibility and will not be accessible to `Python` as they're intended to be internal by design.

By removing the extra layers of conversion we should be seeing a considerable speed-up, alongside all of the other incremental improvements we have made.

Changes:

- Add the pyo3 class/struct `BasisTranslator` that will contain allof the main data used by the transpiler pass to perform its operation.
- Convert the `target_basis` into a set manually from python before sending it into the Rust space.
- Remove the exposure of `basis_search` and `compose_transforms` to python.
- Change `basis_search` so that it accepts references to `HashSet` instances instead of accepting a `HashSet<&str>` instance.
- Change inner method's visibility for `basis_search` and `compose_transform` modules in rust.
- Expose the exception imports from `Target` to the `accelerate` crate.
- Expose `DAGCircuit::copy_empty_like` to the rest of the crates.
- Remove all of the unused imports in the Python-side `BasisTranslator`.

Blockers:
- [ ] #12811

* Fix: Redundancies with serialization methods
- Remove extra copies of `target`, `target_basis`, `equiv_lib`, and `min_qubits`.
- Remove unnecessary mutability in `apply_transforms` and `replace_node`.

* Refactor: Remove `BasisTranslator` struct, use pymethod.
- Using this method avoids the creation of a datastructure in rust and the overhead of deserializing rust structures which can be overly slow due to multiple cloning. With this update, since the `BasisTranslator` never mutates, it is better to not store anything in Rust.

* Lint: Ignore too_many_args flag from clippy on `basis_translator::run()`

* Fix: Remove redundant clone

* Fix: Leverage using `unwrap_operation` when taking op_nodes from the dag.
- Add function signature in `base_run`.

* Update crates/accelerate/src/basis/basis_translator/mod.rs

Co-authored-by: Elena Peña Tapia <[email protected]>

* Adapt to #13164
- Use `QuantumCircuit._has_calibration_for()` when trying to obtain calibrations from a `QuantumCircuit` due to the deprecation of the `Pulse` package.

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
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 priority: high Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants