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

Fix behavior of n_local calling circuits for num_qubits=1 #13523

Merged
merged 17 commits into from
Dec 17, 2024

Conversation

trigpolynom
Copy link
Contributor

@trigpolynom trigpolynom commented Dec 3, 2024

Summary

Fixes #13480.

When circuits that construct n_local were passed num_qubits = 1, an error would be thrown because the default entangling gate, "cx", requires at least 2. The fix involved just checking for num_qubits and setting to an empty array if num_qubits is not greater than 1.

Details and comments

The fix had to be applied to efficient_su2, excitation_preserving, and real_amplitudes. Test methods were added for each changed function.

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Dec 3, 2024
@CLAassistant
Copy link

CLAassistant commented Dec 3, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ethan Gordon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

This approach looks good! I left some small comments, but you can do the same approach for the other functions (like real_amplitudes, ...) 🙂

@trigpolynom trigpolynom changed the title added fix for efficient_su2 Fix behavior of n_local calling circuits for num_qubits=1 Dec 5, 2024
@trigpolynom trigpolynom marked this pull request as ready for review December 5, 2024 04:00
@trigpolynom trigpolynom requested a review from a team as a code owner December 5, 2024 04:00
@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 following people are relevant to this code:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

@Cryoris Cryoris marked this pull request as draft December 5, 2024 08:40
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.

Thanks! There's also pauli_two_design which has the same issue 🙂

@trigpolynom
Copy link
Contributor Author

Thanks! There's also pauli_two_design which has the same issue 🙂

oh shoot did not catch that sorry! Will put in fix tonight!

@Cryoris
Copy link
Contributor

Cryoris commented Dec 10, 2024

Thanks for the updates! The CI seems to be complaining the black should be run, otherwise this LGTM 👍🏻

@trigpolynom
Copy link
Contributor Author

Thanks for the updates! The CI seems to be complaining the black should be run, otherwise this LGTM 👍🏻

should be good to go!

@Cryoris Cryoris marked this pull request as ready for review December 11, 2024 10:36
@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 following people are relevant to this code:

  • @Cryoris
  • @Qiskit/terra-core
  • @ajavadia

@Cryoris Cryoris added this to the 1.3.2 milestone Dec 11, 2024
@Cryoris Cryoris added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Dec 11, 2024
@coveralls
Copy link

coveralls commented Dec 11, 2024

Pull Request Test Coverage Report for Build 12366380330

Details

  • 12 of 12 (100.0%) changed or added relevant lines in 4 files are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.006%) to 88.967%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 6 92.98%
Totals Coverage Status
Change from base Build 12356117829: 0.006%
Covered Lines: 79436
Relevant Lines: 89287

💛 - Coveralls

@trigpolynom trigpolynom requested a review from Cryoris December 13, 2024 02:04
@trigpolynom trigpolynom requested a review from Cryoris December 13, 2024 17:54
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.

LGTM, thanks for the fix @trigpolynom!

@Cryoris Cryoris added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Dec 17, 2024
@Cryoris Cryoris enabled auto-merge December 17, 2024 08:21
@Cryoris Cryoris added this pull request to the merge queue Dec 17, 2024
Merged via the queue into Qiskit:main with commit 82bbcaa Dec 17, 2024
18 checks passed
mergify bot pushed a commit that referenced this pull request Dec 17, 2024
* added fix for efficient_su2

* making fix for real_amplitudes and excitation_preserving

* formatted

* passing tests

* added release notes

* fixed pauli_two_design

* fixed format

* fixed pauli_two_design issue

* fixed unused gate

* Update qiskit/circuit/library/n_local/pauli_two_design.py

Co-authored-by: Julien Gacon <[email protected]>

* addressing change

---------

Co-authored-by: Julien Gacon <[email protected]>
(cherry picked from commit 82bbcaa)
github-merge-queue bot pushed a commit that referenced this pull request Dec 17, 2024
…13575)

* added fix for efficient_su2

* making fix for real_amplitudes and excitation_preserving

* formatted

* passing tests

* added release notes

* fixed pauli_two_design

* fixed format

* fixed pauli_two_design issue

* fixed unused gate

* Update qiskit/circuit/library/n_local/pauli_two_design.py

Co-authored-by: Julien Gacon <[email protected]>

* addressing change

---------

Co-authored-by: Julien Gacon <[email protected]>
(cherry picked from commit 82bbcaa)

Co-authored-by: trigpolynom <[email protected]>
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 Community PR PRs from contributors that are not 'members' of the Qiskit repo stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

efficient_su2 & co should handle the num_qubits=1 case
5 participants