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.

Sign up for GitHub

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

Jump to bottom

BackendEstimator support BackendV2 without coupling_map #10956

Merged
merged 11 commits into from
Oct 11, 2023
Merged

BackendEstimator support BackendV2 without coupling_map #10956

merged 11 commits into from
Oct 11, 2023

Conversation

ikkoham
Copy link
Contributor

@ikkoham ikkoham commented Oct 3, 2023 •
edited
Loading

Summary

This error occurs when Aer is upgraded to BackendV2 because the BackendV2 transpile automatically puts in ancilla qubits.
This PR is to fix.

As a side effect, we have avoided calling unnecessary transpile passes, which should improve performance.

Details and comments

@ikkoham ikkoham added performance Changelog: Bugfix Include in the "Fixed" section of the changelog mod: primitives Related to the Primitives module labels Oct 3, 2023
@ikkoham ikkoham requested a review from mtreinish October 3, 2023 14:40
@ikkoham ikkoham requested review from t-imamichi and a team as code owners October 3, 2023 14:40
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core
  • @ajavadia
  • @ikkoham
  • @levbishop
  • @t-imamichi

@t-imamichi
Copy link
Member

Thanks. Could you add a unit test of BackendV2?

@ikkoham
Copy link
Contributor Author

ikkoham commented Oct 4, 2023 •
edited
Loading

It is difficult to make a test that fails because there is no BackendV2 that does not have a coupling map now. I will think about it.

I found FakeBackendSimple.

@ikkoham ikkoham added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Oct 4, 2023
This was referenced Oct 4, 2023
Merged
Closed
@ikkoham ikkoham linked an issue Oct 5, 2023 that may be closed by this pull request
BackendEstimator._transpile fails for BackendV2 backends #10970
Closed
@ikkoham ikkoham requested a review from t-imamichi October 6, 2023 05:46
@t-imamichi
Copy link
Member

Thank you for adding a test. Could you write a reno about the support of BackendV2 without coupling_map?

@ikkoham ikkoham added this to the 0.25.3 milestone Oct 10, 2023
t-imamichi
t-imamichi approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@t-imamichi t-imamichi left a comment

Choose a reason for hiding this comment

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

LGTM

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6478284986

  • 20 of 21 (95.24%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.009%) to 87.019%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/primitives/backend_estimator.py 20 21 95.24%
Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 93.75%
crates/qasm2/src/lex.rs 4 91.16%
Totals Coverage Status
Change from base Build 6474290134: 0.009%
Covered Lines: 74181
Relevant Lines: 85247
💛 - Coveralls

@t-imamichi t-imamichi enabled auto-merge October 11, 2023 06:07
@t-imamichi t-imamichi added this pull request to the merge queue Oct 11, 2023
Merged via the queue into Qiskit:main with commit be2cebc Oct 11, 2023
13 checks passed
mergify bot pushed a commit that referenced this pull request Oct 11, 2023
@ikkoham @mergify
BackendEstimator support BackendV2 without coupling_map (#10956)
acbb001 
* BackendEstimator support BackendV2 without coupling_map

* revert unnecessary type

* add unittest

* lint

* refactoring

* add release note

---------

Co-authored-by: Takashi Imamichi <[email protected]>
(cherry picked from commit be2cebc)
@mergify mergify bot mentioned this pull request Oct 11, 2023
Merged
github-merge-queue bot pushed a commit that referenced this pull request Oct 11, 2023
@mergify @ikkoham
BackendEstimator support BackendV2 without coupling_map (#10956) (#11006
af48b85 
)

* BackendEstimator support BackendV2 without coupling_map

* revert unnecessary type

* add unittest

* lint

* refactoring

* add release note

---------

Co-authored-by: Takashi Imamichi <[email protected]>
(cherry picked from commit be2cebc)

Co-authored-by: Ikko Hamamura <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-imamichi t-imamichi t-imamichi approved these changes

@mtreinish mtreinish Awaiting requested review from mtreinish

Assignees
No one assigned
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: primitives Related to the Primitives module performance stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Milestone
0.25.3
Development

Successfully merging this pull request may close these issues.

BackendEstimator._transpile fails for BackendV2 backends
4 participants
@ikkoham @qiskit-bot @t-imamichi @coveralls