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 pulse and calibration management related code #1511

Merged
merged 28 commits into from
Mar 2, 2025

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Feb 17, 2025

This change prepares the package for Qiskit 2.0 by removing all features related to Qiskit Pulse which is removed in Qiskit 2.0 (see for example Qiskit#13872). It includes several related changes:

  • Remove all experiments that require pulse schedules to run on a backend with a set of standard basis gates. The set of removed experiments includes Rabi and QubitSpectroscopy for example. Additionally, analysis classes used only the removed experiments are also removed. These classes should have all emitted deprecation warnings in the previous release.
  • Remove all code related pulse calibration management, including the Calibrations and BasisGateLibrary classes and the calibration experiment classes. These classes should have all emitted deprecation warnings in the previous release.
  • Deprecate PulseBackend and drop qiskit-dynamics as a dependency of qiskit-experiments[extras]. This is a last dependency on Qiskit Pulse that is not fully dropped yet because it was not previously deprecated.
  • Convert the jupyter-execute cells in the docs to use Qiskit Aer or mock backends instead of using PulseBackend.
  • Remove pulse support from other experiments and classes that have not been removed including methods of BackendData and BackendTiming like pulse_time and the feature of MultiStateDiscrimination that allowed it to accept pulse schedules for exciting higher qubit states. Also, remove the ability to serialize and deserialize pulse schedule blocks with ExperimentEncoder and ExperimentDecoder.
  • Deprecate FineZXAmplitude as it can not be easily used without a pulse calibration to implement an RZX rotation.
  • Update examples in the documentation that previously used removed experiments to use other experiments that have not been removed.
  • Remove support for custom pulse calibrations from randomized benchmarking experiments and ParallelExperiment.
  • Add a private ZZRamseyTestBackend for the ZZRamsey tests and documentation that is similar to T2HahnBackend to provide more consistency with the other experiment examples that were updated not to use PulseBackend.

Note that a few small pulse features might have been removed without a release cycle of deprecation warnings. Since the IBM Quantum service was the main use case of the Qiskit Pulse features and it has already stopped accepting jobs with Qiskit Pulse calibrations, it was deemed better to do a clean removal of all pulse features than to adhere strictly to the deprecation policy.

The changes here focus on the Qiskit Pulse related updates needed for compatibility with Qiskit 2.0. Some further follow up will also be necessary to address other incompatibilities with Qiskit 2.0.

@wshanks
Copy link
Collaborator Author

wshanks commented Feb 17, 2025

Still to be done:

  • Release notes
  • Remove SingleTransmonTestBackend or deprecate it and protect it from import errors

Could be deferred to later PR:

  • Remove pulse warning filters and address any remaining warnings
  • Update use of qiskit.qobj (all should be removed but MeasLevel/MeasReturn are being moved under qiskit.result).
  • Remove restless support

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 for the hard work. The PR seems to cover all necessary changes we need to make for Qiskit 2.0.

We also need to consider the migration of this tutorial:
https://qiskit-community.github.io/qiskit-dynamics/tutorials/dynamics_backend.html

Qiskit Dynamics decided to pin Qiskit core version < 2.0, so that code will continue to work with their simulator. Maybe we can just add warning section in above tutorial. Any thoughts? @DanPuzzuoli

from qiskit_experiments.framework import Options


class ResonanceAnalysis(curve.CurveAnalysis):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can keep analysis itself because it doesn't depend on the pulse module. From the maintenance viewpoint, it's reasonable to remove unused code though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I am not sure if there is something more we could do to preserve the old code... For now, I will personally just remember that the 0.8 release had all the pulse related code. I would rather remove it for now and pull it back later when it is useful. Otherwise, I think it will break over time as things change and it is not updated or it will add overhead as it is kept up to date without being used.

@@ -18,7 +18,6 @@ class unifies data access for various data fields.
import warnings
from qiskit.providers.models import PulseBackendConfiguration # pylint: disable=no-name-in-module
from qiskit.providers import BackendV1, BackendV2
from qiskit.utils.deprecation import deprecate_func
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this backend_data class? Since Qiskit will drop the V1 model, we don't need to maintain this abstraction layer. It would make sense to keep this for future models.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We definitely need to make changes to BackendData so that it does not fail for deprecated Qiskit code references. I need to clean this PR up a little bit. The theme here is "remove the deprecated pulse and calibration experiments" and I think that is big enough for one PR. There is still some work to do to make sure the tests don't generate other deprecation warnings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, the pulse related code was fairly extensive and is somewhat already not useful since IBM disabled pulse in the API already. For other Qiskit 2.0 changes like removing BackendV1, we might be able to provide a smoother transition between Qiskit 1 and Qiskit 2 without much effort (so we might not need to do a full removal right now).

error = 0.05
x_error = coherent_unitary_error(RXGate(error).to_matrix())
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 neat :)

@@ -94,14 +95,10 @@ def _default_experiment_options(cls) -> Options:

Experiment Options:
n_states (int): The number of states to discriminate.
schedules (dict): A dictionary of the schedules for the gates in the experiment. Each key is
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this experiment work? Qiskit doesn't have x12 gate in the builtin library.

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 still works for 2 states. It would work for more states if a Qiskit backend provided instructions for exciting higher states. That doesn't seem too hard to imagine, so I thought we could keep this.

@@ -486,21 +484,6 @@ def _transpiled_circuits(self) -> List[QuantumCircuit]:
for gate_1q in self.experiment_options.one_qubit_basis_gates:
instructions.append((gate_1q, (q,)))

common_calibrations = defaultdict(dict)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can also remove if-clause for BackendV2 above along this this code block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in d21b721

@DanPuzzuoli
Copy link

Thanks for the hard work. The PR seems to cover all necessary changes we need to make for Qiskit 2.0.

We also need to consider the migration of this tutorial: https://qiskit-community.github.io/qiskit-dynamics/tutorials/dynamics_backend.html

Qiskit Dynamics decided to pin Qiskit core version < 2.0, so that code will continue to work with their simulator. Maybe we can just add warning section in above tutorial. Any thoughts? @DanPuzzuoli

I guess with pinning qiskit to < 2.0 within Dynamics, will we also need to pin qiskit experiments to some version? Not sure how the internals would work without pulse.

@wshanks
Copy link
Collaborator Author

wshanks commented Feb 24, 2025

I guess with pinning qiskit to < 2.0 within Dynamics, will we also need to pin qiskit experiments to some version? Not sure how the internals would work without pulse.

Right, that was my thought for the near term future. Since Qiskit 2.0 is coming out pretty soon, I just want to pull out anything that breaks with it for now. There is a plan to move pulse to dynamics and maybe we can put back some of the pulse related experiments and calibration helpers, or maybe it will make sense to move them to Dynamics as well to be closer to the pulse stuff. Since Qiskit 2.0 is removing things like QuantumCircuit.calibrations, I think it will take some rework for the calibration experiments to work with Qiskit 2.0 even if there is pulse code in Dynamics.

@DanPuzzuoli
Copy link

I guess with pinning qiskit to < 2.0 within Dynamics, will we also need to pin qiskit experiments to some version? Not sure how the internals would work without pulse.

Right, that was my thought for the near term future. Since Qiskit 2.0 is coming out pretty soon, I just want to pull out anything that breaks with it for now. There is a plan to move pulse to dynamics and maybe we can put back some of the pulse related experiments and calibration helpers, or maybe it will make sense to move them to Dynamics as well to be closer to the pulse stuff. Since Qiskit 2.0 is removing things like QuantumCircuit.calibrations, I think it will take some rework for the calibration experiments to work with Qiskit 2.0 even if there is pulse code in Dynamics.

We've actually decided to not move pulse to Dynamics, and instead just permanently pin the qiskit version for users doing pulse simulation. In the end Dynamics doesn't really depend in a meaningful way on anything in qiskit outside of pulse, so moving it to Dynamics would result in a lot of unnecessary work.

@wshanks
Copy link
Collaborator Author

wshanks commented Feb 25, 2025

We've actually decided to not move pulse to Dynamics, and instead just permanently pin the qiskit version for users doing pulse simulation. In the end Dynamics doesn't really depend in a meaningful way on anything in qiskit outside of pulse, so moving it to Dynamics would result in a lot of unnecessary work.

Ah, okay, I hadn't seen that decision. So that aligns even more with what is done here -- just drop the pulse stuff and if you want it you can pin to version 0.8 of qiskit-experiments. I don't think it makes sense to try to maintain the code for a discontinued version of Qiskit. We could until Qiskit 1.x goes end of life but we are not planning on adding new features for the pulse stuff, so just pinning to qiskit-experiments should be the same effect.

@wshanks wshanks changed the title WIP: Qiskit2 removals Remove pulse and calibration management related code Feb 27, 2025
@wshanks wshanks marked this pull request as ready for review February 27, 2025 21:09
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. Code change looks good to me. I wonder if we can release 1.0 to sync the release strategy with Qiskit core, as we are also introducing immediate breaking API change with this PR.

@wshanks
Copy link
Collaborator Author

wshanks commented Feb 28, 2025

Hmm, which breaking API changes are you thinking of? Largely, this PR removes things that were deprecated in the previous release. There are some things removed that were not issuing deprecation warnings before like ParameterValue but the release notes said that all of the calibration support was deprecated and the other calibration classes don't make much sense to use without Calibrations which did have a deprecation warning. Also, support for pulse gate calibrations was removed from parallel experiment and the randomized benchmarking experiments without deprecation warnings but those are still somewhat covered by the release note saying all calibration support was deprecated. Also, since the IBM API does not accept circuits with pulse gate calibrations any more, I don't feel too bad about some breaking changes in that part of the code (it was unusable any way outside of simulation with qiskit-dynamics). Do you see other breaking changes?

I think incrementing to 1.0 would be more a statement of stability than indicating breaking changes. I think our policy of trying to give one minor release of deprecation warning before removal is more in keeping with how much stability we want to aim for right now?

@nkanazawa1989
Copy link
Collaborator

nkanazawa1989 commented Mar 2, 2025

Yes, I agree. But from your discussion above, I was just thinking it might be clearer for the users if we drop all of these components as a part of major release. The user can just pin with < 1.0 if they want pulse experiments. Also we don't actively develop the repo and code looks mature.

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 2, 2025

Hmm, I am not sure about the status of development on the repo. The size of the team working on it is much smaller than it was and so the pace of change is also slower and it is more stable in that sense, but the code is still in use by some of the maintainers and I don't think they want to commit to more stability. For example if a feature breaks that is not used by the maintainers the response might be to remove it rather than fix it.

@wshanks wshanks added this pull request to the merge queue Mar 2, 2025
Merged via the queue into qiskit-community:main with commit eecce03 Mar 2, 2025
11 checks passed
@wshanks wshanks deleted the qiskit2-removals branch March 6, 2025 05:28
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.

3 participants