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

Improve clifford_decompose_greedy algorithm` #12560

Closed

Conversation

alexanderivrii
Copy link
Contributor

@alexanderivrii alexanderivrii commented Jun 12, 2024

Summary

This PR improves the performance of the clifford_decompose_greedy algorithm by noticing that evolving a Pauli of the form I..IXI...I or of the form I..IZI...I by a Clifford C is essentially equivalent to grabbing a row from C's tableau representation, except that the phase needs to be multiplied by 2. (In fact, we can think of the tableau of C as representing how C acts on the basis Pauli strings I..IXI...I and I..IZI...). As a sanity check, I have also verified this experimentally on random cliffords of various sizes.

Details and comments

Here is the data for running the synthesis algorithm for 10 random Cliffords of various sizes.
Before this PR:

Num-qubits 5 => total time 0.18
Num-qubits 10 => total time 0.70
Num-qubits 15 => total time 1.91
Num-qubits 20 => total time 4.91
Num-qubits 50 => total time 39.91
Num-qubits 100 => total time 288.60

With this PR:

Num-qubits 5 => total time 0.09
Num-qubits 10 => total time 0.32
Num-qubits 15 => total time 0.73
Num-qubits 20 => total time 1.51
Num-qubits 50 => total time 13.63
Num-qubits 100 => total time 101.92

The runtime improvement is at least 2x.

@alexanderivrii alexanderivrii requested review from ShellyGarion and a team as code owners June 12, 2024 21:45
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9490163205

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 16 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 89.588%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 92.88%
crates/qasm2/src/parse.rs 12 97.15%
Totals Coverage Status
Change from base Build 9483656494: -0.02%
Covered Lines: 62517
Relevant Lines: 69783

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 13, 2024

Pull Request Test Coverage Report for Build 9496416834

Details

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.009%) to 89.595%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 93.13%
crates/qasm2/src/parse.rs 6 97.61%
Totals Coverage Status
Change from base Build 9483656494: -0.009%
Covered Lines: 62512
Relevant Lines: 69772

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 13, 2024

Pull Request Test Coverage Report for Build 9498328834

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.

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • 14 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.01%) to 89.591%

Files with Coverage Reduction New Missed Lines %
qiskit/circuit/parametervector.py 3 94.44%
crates/qasm2/src/lex.rs 5 92.37%
crates/qasm2/src/parse.rs 6 97.61%
Totals Coverage Status
Change from base Build 9483656494: -0.01%
Covered Lines: 62504
Relevant Lines: 69766

💛 - Coveralls

@ShellyGarion ShellyGarion added Changelog: New Feature Include in the "Added" section of the changelog synthesis labels Jun 18, 2024

# Calculate the adjoint of clifford_current without the phase
Copy link
Member

Choose a reason for hiding this comment

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

is this comment still correct? do you calculate the adjoint or the evolved cliffords directly?

Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

LGTM, please look at the comments again, since some of them are irrelevant now.
e.g.:

# Compute the decoupling operator of cliff_ox and cliff_oz

there is no cliff_ox or cliff_oz

@alexanderivrii
Copy link
Contributor Author

The point of this PR was to resolve a certain inefficiency in clifford_decompose_greedy. I am closing this PR, since it is now completely superseded by #12601.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog synthesis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants