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

Support ECR gates in Pauli.evolve(QuantumCircuit) #12095

Merged
merged 10 commits into from
Apr 18, 2024

Conversation

aeddins-ibm
Copy link
Contributor

Summary

Should fix #12086 and also #12093.

Details and comments

- Adds support for Pauli (and related classes) to evolve through ECR gates encountered in a quantum circuit.
- Also moved the dicts of special-case gates (`basis_1q`, `basis_2q`, `non-clifford`) outside the subroutine definition. They are now just after the `_evolve_*()` functions they reference.
- Should fix qiskit issue Qiskit#12093
- Bug happened after converting circuit to instruction, which AFAICT was not necessary. Now if input is a QuantumCircuit, that part of the code is bypassed.
- Removed creation of a look-up dict of bit locations, since `QuantumCircuit.find_bit` already provides one.
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Mar 28, 2024
@coveralls
Copy link

coveralls commented Mar 28, 2024

Pull Request Test Coverage Report for Build 8648164561

Details

  • 22 of 34 (64.71%) changed or added relevant lines in 1 file are covered.
  • 19 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.02%) to 89.375%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/quantum_info/operators/symplectic/base_pauli.py 22 34 64.71%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.03%
crates/qasm2/src/lex.rs 6 92.11%
crates/qasm2/src/parse.rs 12 97.15%
Totals Coverage Status
Change from base Build 8646610213: -0.02%
Covered Lines: 60318
Relevant Lines: 67489

💛 - Coveralls

@ShellyGarion ShellyGarion added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) and removed Community PR PRs from contributors that are not 'members' of the Qiskit repo labels Mar 28, 2024
@ShellyGarion ShellyGarion self-assigned this Mar 28, 2024
@aeddins-ibm aeddins-ibm marked this pull request as ready for review March 28, 2024 14:56
@aeddins-ibm aeddins-ibm requested review from ikkoham and a team as code owners March 28, 2024 14:56
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Mar 28, 2024
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @ikkoham

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.

@aeddins-ibm - thanks for finding these bugs and fixing them!
Could you perhaps add a test that shows that #12093 is solved?
Could you also add some release notes?

@ShellyGarion ShellyGarion removed the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Mar 31, 2024
@aeddins-ibm
Copy link
Contributor Author

Hi @ShellyGarion, I have added a test and also a release note. I haven't written release notes before so just tried to follow what others look like. In particular is it OK that 1) there's no max-characters-per-line in the note, and 2) I just labeled it "fixes" rather than something more specific like "fixes_quantuminfo" (if that's even a thing people write)? Thanks

@aeddins-ibm
Copy link
Contributor Author

The automated tests above failed with the error message
ERROR: Could not build wheels for qiskit-aer, which is required to install pyproject.toml-based projects

I don't know if this is somehow caused by this PR, or if it's some unrelated failure. Does anyone have a suggestion how to proceed? The commit where the failure started showing up looks innocuous to me.

@mtreinish
Copy link
Member

The automated tests above failed with the error message ERROR: Could not build wheels for qiskit-aer, which is required to install pyproject.toml-based projects

I don't know if this is somehow caused by this PR, or if it's some unrelated failure. Does anyone have a suggestion how to proceed? The commit where the failure started showing up looks innocuous to me.

qiskit-aer released this morning and didn't build wheels (i.e. precompiled binaries) for 3.8 by mistake. I manually retriggered a build for py3.8; assuming that works it should resolve itself soon. If it doesn't then I'll push up a PR to exclude the 0.14.0 release until it can be resolved.

@mtreinish
Copy link
Member

The manual build for 3.8 on aer completed and the wheels are on pypi now, so I've retriggered the failed job and it should hopefully pass now.

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.

Generally, this PR looks good to me.

Perhaps you could also add ECRGate to test_phase_dtype_evolve_clifford in test_pauli_list ?

The release notes look fine to me, but I prefer to let someone from Qiskit Core review them too.

@ShellyGarion ShellyGarion removed the request for review from ikkoham April 2, 2024 12:19
ShellyGarion
ShellyGarion previously approved these changes Apr 10, 2024
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, thanks @aeddins-ibm for finding this bug and fixing it!

@ShellyGarion
Copy link
Member

It seems that now there is a conflict that should be resolved. @aeddins-ibm - could you please handle it?

@ShellyGarion ShellyGarion added this pull request to the merge queue Apr 18, 2024
Merged via the queue into Qiskit:main with commit d10f9a0 Apr 18, 2024
12 checks passed
@jakelishman jakelishman self-assigned this Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators)
Projects
Status: Done
6 participants