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

Update FakeBackend to use BackendV2 #1102

Merged
merged 3 commits into from
Mar 27, 2023
Merged

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Mar 24, 2023

Summary

This change set updates the FakeBackend used by some tests to derive from BackendV2 rather than BackendV1

Details and comments

When combined with #900 which updates some calibration tests and #1040 which updates T2HahnBackend, this change results in almost all the tests in the test suite using BackendV2. The remaining tests using a Backend that is not BackendV2 use the AerSimulator which will be updated to BackendV2 in the future (tracked in Qiskit/qiskit-aer#1681). The check for backend type was performed by merging the calibration and T2HahnBackend branches into this branch and running the tests with BaseExperiment.run(), BaseExperiment._set_backend(), and Calibrations.from_backend() updated to error if the backend argument was not BackendV2 or AerSimulator and confirming that all the tests passed.

@wshanks wshanks added the Changelog: None Do not include in changelog label Mar 24, 2023
}
default_config.update(**config)
super().__init__(QasmBackendConfiguration(**default_config))
target = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Overriding abstract property method in this way seems hacky. I prefer to define a protected member and return it through property method as long as the base class provides such mechanism.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that is more proper. I copied this from MinimalBackend, so I updated both classes in this PR now. I think for MinimalBackend I had been thinking I wanted to do backend.target = Target() in tests to test out different Target configurations but that wasn't a good reason to deviate from it being a property.

Copy link
Collaborator

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

Thanks @wshanks . This PR looks good to go.

@wshanks wshanks added this pull request to the merge queue Mar 27, 2023
Merged via the queue into qiskit-community:main with commit 95b9274 Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants