Oxidize synth_permutation_acg
#12543
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Raynel Sanchez <[email protected]>
Co-authored-by: Raynel Sanchez <[email protected]>
maybe faster possible...
Cryoris
added
the
Rust
Cryoris
requested review from
alexanderivrii,
ShellyGarion and
a team
as code owners
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 9463371888Details
💛 - Coveralls |
ElePT
reviewed
Jun 11, 2024
Pull Request Test Coverage Report for Build 9549394978Warning: 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.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9579566605Details
💛 - Coveralls |
|
Let's merge #12605 first to have the right directory structure. |
Pull Request Test Coverage Report for Build 9610372304Warning: 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.
Details
💛 - Coveralls |
ShellyGarion
requested changes
Jun 30, 2024
Pull Request Test Coverage Report for Build 9761267322Warning: 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.
Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9762022533Warning: 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.
Details
💛 - Coveralls |
ShellyGarion
approved these changes
Jul 2, 2024
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Jul 2, 2024
After the recently merged Qiskit#12543 when working with Rust 1.79 cargo fmt makes a small formatting change that rust 1.70 wouldn't and clippy makes flags a &Vec<_> that should be a slice &[_] instead. This commit makes these two small chagnes so they're not an issue for people building with the latest stable version of rust, not just our MSRV.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 2, 2024
After the recently merged #12543 when working with Rust 1.79 cargo fmt makes a small formatting change that rust 1.70 wouldn't and clippy makes flags a &Vec<_> that should be a slice &[_] instead. This commit makes these two small chagnes so they're not an issue for people building with the latest stable version of rust, not just our MSRV.
ElePT
added
the
Changelog: New Feature
Procatv
pushed a commit
to Procatv/qiskit-terra-catherines
that referenced
this pull request
Aug 1, 2024
* Move utility functions _inverse_pattern and _get_ordered_swap to Rust * fix formatting and pylint issues * Changed input type to `PyArrayLike1<i64, AllowTypeChange>` * Refactor `permutation.rs`, clean up imports, fix coverage error * fix docstring for `_inverse_pattern` Co-authored-by: Raynel Sanchez <[email protected]> * fix docstring for `_get_ordered_swap` Co-authored-by: Raynel Sanchez <[email protected]> * remove pymodule nesting * remove explicit `AllowTypeChange` * Move input validation out of `_inverse_pattern` and `_get_ordered_swap` * oxidization attempt no. 1 * working version! maybe faster possible... * move more into rust & fix clones * layouting & comments * dangling comment * circuit construction in rust * remove dangling code * more lint * add reno * drop redundant Ok(expect()) * Implements Shelly's suggestions * simplify code a little --------- Co-authored-by: jpacold <[email protected]> Co-authored-by: Raynel Sanchez <[email protected]>
Procatv
pushed a commit
to Procatv/qiskit-terra-catherines
that referenced
this pull request
Aug 1, 2024
After the recently merged Qiskit#12543 when working with Rust 1.79 cargo fmt makes a small formatting change that rust 1.70 wouldn't and clippy makes flags a &Vec<_> that should be a slice &[_] instead. This commit makes these two small chagnes so they're not an issue for people building with the latest stable version of rust, not just our MSRV.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #12226. This could in future be further improved by constructing the circuit in Rust directly.
Details and comments
A small benchmark for 1000 qubit circuit on random circuits with depth 1000 gives
It seems that 99% of this improvement comes from constructing the circuit in Rust, not from porting doing the permutation logic.
I'm happy about any comments or possible improvements, as this is my first Rust PR 🙂