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

Remove remaining code usage deprecated in Qiskit 0.46 #1384

Merged
merged 7 commits into from
Feb 3, 2024

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Feb 1, 2024

This change adjusts for several remaining changes in Qiskit 1.0:

  • qiskit.providers.basicaer was removed in Qiskit 1.0 (see Rename BasicAer to BasicProvider and refactor interface to follow BackendV2 Qiskit/qiskit#11422). Here its use is replaced with qiskit-aer.

  • qiskit.providers.fake_provider.FakeJob was removed in Qiskit 1.0 (see Remove provider-specific fake backends,FakeProvider class and related tools in 1.0 Qiskit/qiskit#11376). Here the FakeJob already in qiskit_experiments.test is used instead. This update was missed in f22bb7b which switched the other fake_provider usage.

  • Replace qiskit.provider.FakeProvider with a custom subclass of ProviderV1. FakeProvider was moved out of Qiskit 1.0 to qiskit-ibm-runtime.

  • calibration/test_rabi.py was modified to make the data for a test of bad data worse as the test started fitting the data as good with the original parameters and qiskit-aer in place of basicaer.

  • In the readout mitigation manual, a use of plot_histogram was exchanged for one of plot_distribution.

  • Guard FakeBackendV2 import that was removed in Qiskit 1.0. Try to import FakeBackendV2 from qiskit-ibm-runtime as well.

Also, this change unpins qiskit which had to be pinned in f22bb7b until these fixes could be merged since 0.46.0 was released with deprecation warnings for some of the usage changed here.

@wshanks
Copy link
Collaborator Author

wshanks commented Feb 1, 2024

@coruscating This fixes the tests against Qiskit main which I noticed had started failing again.

# Change rotation angle with square root of amplitude so that
# population versus amplitude will not be sinusoidal and the fit will
# be bad.
thetas = np.sqrt(np.linspace(0.0, 4 * np.pi**2, 31))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seemed like this test chose somewhat arbitrary input data to be "bad" but changing simulators made it start fitting as good again. I made the data worse so that it should more reliably fit as bad.

@wshanks wshanks requested a review from coruscating February 2, 2024 04:07
@wshanks wshanks changed the title Replace qiskit.providers.basicaer test usage with qiskit-aer Replace qiskit.providers.basicaer test usage with qiskit-aer and import FakeJob from qiskit-ibm-runtime Feb 2, 2024
@wshanks wshanks changed the title Replace qiskit.providers.basicaer test usage with qiskit-aer and import FakeJob from qiskit-ibm-runtime Remove remaining qiskit.providers usage of classes deprecated in Qiskit 0.46 Feb 2, 2024
@wshanks wshanks changed the title Remove remaining qiskit.providers usage of classes deprecated in Qiskit 0.46 Remove remaining code usage deprecated in Qiskit 0.46 Feb 2, 2024
Copy link
Collaborator

@coruscating coruscating left a comment

Choose a reason for hiding this comment

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

This works with Qiskit 0.46, but FakeBackendV2 has also been removed in 1.0 (the code is still there but the import path is gone). I think this import was probably meant to be updated in #1369:

https://github.com/Qiskit-Extensions/qiskit-experiments/blob/f22bb7b67893a1b75f7d0f1fecdb0bd63284e244/qiskit_experiments/framework/backend_data.py#L20

@@ -33,7 +34,7 @@
AnalysisStatus,
)
from qiskit_experiments.test.fake_backend import FakeBackend
from qiskit_experiments.database_service import Qubit
from qiskit_experiments.test.utils import FakeJob
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good for now. I noticed our FakeJob is implemented differently from the removed FakeJob and also from current Jobs. For example, the qiskit-ibm-runtime Job doesn't have time_per_step() implemented currently. If we're using our own FakeJob going forward, we should make sure it matches the external specs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That sounds good. If possible, I would like to see us push more for external specs. The Qiskit base classes are minimal and we rely on a lot of private attributes of the provider classes in a way that seems very brittle.

@wshanks wshanks force-pushed the basicaer branch 2 times, most recently from 6558622 to 632f793 Compare February 3, 2024 05:11
@wshanks
Copy link
Collaborator Author

wshanks commented Feb 3, 2024

Maybe we should remove that now to try to get the cron tests working again.

I added a change for it. I think the cron tests will still fail because we need qiskit-ibm-runtime which still depends on qiskit-ibm-provider in its latest release and qiskit-ibm-provider is not compatible with qiskit 1.0. If we don't want to wait for qiskit-ibm-runtime, we might need to fork it, patch out the qiskit-ibm-provider part (sloppily since we just need the fake backends), and install from the fork until qiskit-ibm-runtime can properly remove the qiskit-ibm-provider parts.

I think we should deprecate BackendData.is_simulator. It is not used in the repo and I am not sure it is that useful to users. The online simulators are deprecated and besides that the user should be able to tell from context if a backend is a simulator (they won't be iterating over a list from a provider with a mix of simulators and hardware backends).

Copy link
Collaborator

@coruscating coruscating left a comment

Choose a reason for hiding this comment

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

LGTM!

@coruscating coruscating added this pull request to the merge queue Feb 3, 2024
Merged via the queue into qiskit-community:main with commit a87f68c Feb 3, 2024
11 checks passed
@wshanks wshanks deleted the basicaer branch February 3, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants