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

SingleQubitOperation trait and speedup to Optimize1qGatesDecomposition pass #14020

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

Conversation

alexanderivrii
Copy link
Member

Summary

This PR implements a new trait SingleQubitOperation for StandardGate, PyGate, UnitaryGate and PackedOperation. The trait currently defines a single method fn get_mat_static_1q(&self, params: &[Param]) -> Option<[[Complex64; 2]; 2]> which returns a matrix for single-qubit gates (and None otherwise). This is analogous to the matrix method in Operation except that it's tuned for single-qubit matrices, allowing to represent these as [[Complex64; 2]; 2] instead of Array2<Complex64>.

This is used to speed up matrix multiplication inside optimize_1q_gates_decomposition, leading to an about 10% performance improvement for the Optimize1qGatesDecomposition transpiler pass.

@alexanderivrii alexanderivrii added this to the 2.1.0 milestone Mar 14, 2025
@alexanderivrii alexanderivrii requested a review from a team as a code owner March 14, 2025 10:07
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@alexanderivrii alexanderivrii added performance Rust This PR or issue is related to Rust code in the repository labels Mar 14, 2025
Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

I'm no Rust expert, so it's very possible I'm wrong, but I'm not sure this is how Trait should be used here. I would expect something like a SingleQubitOperation would only be implemented for single-qubit gates, but here it is implemented for all standard gates. Typically I expect a trait to allow me to write something like

fn my_special_function_for_1q(gate: impl SingleQubitOperation) {
  // now I am certain that ``gate`` is indeed a 1q operation
}

but this is not the case, since any StandardGate variant fulfills this trait (and just returns a None matrix).

If I understand correctly, the thing you want to enable is for matrix to return a slice instead of an owned Array, right? If that's so, would another solution be to add something like a matrix_view or matrix_as_slice to the Operation interface instead?

@coveralls
Copy link

coveralls commented Mar 14, 2025

Pull Request Test Coverage Report for Build 13909937100

Details

  • 90 of 166 (54.22%) changed or added relevant lines in 3 files are covered.
  • 6 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.07%) to 88.015%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/packed_instruction.rs 7 14 50.0%
crates/circuit/src/operations.rs 66 135 48.89%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/unitary_synthesis.rs 1 94.69%
crates/qasm2/src/expr.rs 1 94.23%
crates/qasm2/src/lex.rs 4 92.73%
Totals Coverage Status
Change from base Build 13905217637: -0.07%
Covered Lines: 72678
Relevant Lines: 82575

💛 - Coveralls

@alexanderivrii alexanderivrii requested a review from Cryoris March 17, 2025 16:05
@alexanderivrii
Copy link
Member Author

alexanderivrii commented Mar 17, 2025

If I understand correctly, the thing you want to enable is for matrix to return a slice instead of an owned Array, right? If that's so, would another solution be to add something like a matrix_view or matrix_as_slice to the Operation interface instead?

Thanks @Cryoris. As we have discussed offline, there are indeed several different ways to implement the required functionality, with some decisions to be made about (1) should this be a part of the Operation trait or should this be a part of a different trait, (2) should this new trait be called SingleQubitOperation or something like MatrixRepresentation, and (3) do we want to have a method that returns Some([[Complex64; 2]; 2]) for a single-qubit gate and None otherwise (as implemented right now), or do we want to have a method that returns a matrix in all possible cases.

I don't think we can implement different traits for different variants of the same enum, i.e. SingleQubitOperation for StandardGate::X and TwoQubitOperation for StandardGate:CX, and in particular I don't think we have a good alternative for (3): for performance considerations it's probably not worth to wrap the returned matrix into some other enum. The current decisions for (1) and (2) are based on an offline discussion with @mtreinish and transitively with @jakelishman; Matthew or Jake, could you please comment on this, especially how do you see the bigger picture of how the SingleQubitOperation trait might evolve in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants