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 fake backend kwargs #6743

Merged
merged 12 commits into from
Jul 14, 2021
Merged

Fix fake backend kwargs #6743

merged 12 commits into from
Jul 14, 2021

Conversation

nonhermitian
Copy link
Contributor

Summary

Fixes #6741

Adds kwargs to backend.run() with noise model.

Details and comments

@nonhermitian nonhermitian requested a review from a team as a code owner July 14, 2021 02:05
@nonhermitian nonhermitian changed the title Fix fake sim kwargs Fix fake backend kwargs Jul 14, 2021
@mtreinish mtreinish added Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable labels Jul 14, 2021
Copy link
Member

@mtreinish mtreinish 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 pushing the quick fix

@jakelishman
Copy link
Member

The test failure that occurred in 06fe680 (and a few before that) looks like it'll bite again - the bug is that in FullQiskitTestCase, the method resolution via testtools for setUp() requires that super().setUp() is called. If running locally without setting QISKIT_TEST_CAPTURE_STREAMS=1, we use BasicQiskitTestCase which resolves directly to unittest, and that doesn't care. Since the other test in this module is currently unconditionally skipped, nothing noticed that its setUp() doesn't called super().setUp() until this test was originally added to the same class.

That said, while its unconditional skip reason no longer seems to be valid (Qiskit/qiskit-aer#741 is fixed and released), the test fails by seemingly using something out-of-date, and I don't yet have the experience with the codebase to know much about that.

@mtreinish
Copy link
Member

@jakelishman can you open an issue about this? I think that's a bunch of improvements we can/should make to the testing harness. At least for this case it's a simple to add the super() call enforcement to the BasicQiskitTestCase class too. But, the way things are split has caused issues in other places too (thinking of: Qiskit/qiskit-aer@fdf2cda ).

@mergify mergify bot merged commit 795e845 into Qiskit:main Jul 14, 2021
mergify bot pushed a commit that referenced this pull request Jul 14, 2021
* fix fake backends not getting kwargs

* add test

* damn missing braket

* move to new class

* rename new class

* I hate lint

* Add release note

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
(cherry picked from commit 795e845)
mergify bot added a commit that referenced this pull request Jul 14, 2021
* fix fake backends not getting kwargs

* add test

* damn missing braket

* move to new class

* rename new class

* I hate lint

* Add release note

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
(cherry picked from commit 795e845)

Co-authored-by: Paul Nation <nonhermitian@gmail.com>
@kdk kdk added this to the 0.19 milestone Nov 15, 2021
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 stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting shots in execute no longer works
4 participants