-
Notifications
You must be signed in to change notification settings - Fork 127
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
Deprecate pulse and restless related experiments and classes #1475
Conversation
* Patch ExperimentData.completion_times to handle Job.time_per_step not existing * Pass noise_model run option through to Sampler * Add support for level 1 data to Sampler execution * Handle case where the Sampler strips circuit metadata from results * Mark some test backends as simulators so the sampler does not try to validate circuits * Change some tests to be more consistent about shots since the Sampler can not handle the requested number of shots differing from the shots in the actual results. * Support level 2 bitstrings in MockIQBackend * Fix PulseBackend returning data as numpy array instead of a list * Pass run options through to backend in T2HahnBackend * Do not set block_for_results timeout to 0 when TEST_TIMEOUT is 0 (TEST_TIMEOUT=0 indicates no timeout, not immediate timeout). * Monkey-patch BackendSamplerV2's run circuits function in the tests so that it does not strip circuit metadata that the test backends need to generate results. * Fix inconsistency between bitstring format and number of qubits in restless test. * Handle case where job class does not have error_message
…nity#1473) These changes could be merged into qiskit-community#1470 (PR could be redirected to that branch). For now I am just opening a PR to see how the tests run in CI.
fb88eaa
to
b5b82fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to go. I added some minor comments. I'll approve this after #1470 is merged. I'm also curious how you globally disabled pulse deprecation in unit tests (I was expecting there would be some modification to tox or pyproject file).
@@ -145,6 +145,14 @@ by a variant of the Hahn-echo pulse sequence [5]_. | |||
|
|||
%matplotlib inline | |||
|
|||
import warnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to filter warning? If we remove the warnings, perhaps it's better to add .. caution::
directive also in the tutorial to tell the experiment will be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we have to filter the warnings because of the way jupyter-sphinx works. It converts all Python warnings into Sphinx warnings and we run Sphinx in strict mode where warnings cause the build to fail. I wish we could change that behavior, though on the other hand it usually looks bad to have warnings in a tutorial. I can add a .. caution::
. I was not sure how far to go in general with deprecating things. To follow the policy, I think I should deprecate the analysis classes as well? I wonder if there are other things under the experiments that are technically public and don't make sense to keep when the experiments are removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the policy we should deprecate everything in public. However, since experiment internally instantiates analysis, user will see the warning twice. I think this is a bit annoying. I think it's better to add some deprecation message to the tutorial since the tutorial will be also removed with experiment classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there were not too many cases, I tried just catching the analysis warnings in the deprecated experiments so the experiment only emits one warning but the analysis class will still emit a warning otherwise (and get the warning that shows up in the docs).
7e50aa3
to
0501765
Compare
This change deprecates the experiments that rely on scanning the parameters of pulses in pulse gate calibrations. Qiskit 2.0 will remove support for pulse gate calibrations, making these experiments impossible to run. The `Calibrations` and `BasisGateLibrary` classes are also deprecated since they have no use without pulse gate calibrations to track. It is planned that Qiskit Pulse will be moved to Qiskit Dynamics and perhaps the experiments and calibrations can be adapted to that use case for calibrating simulated experiments. For now though, this code is removed from Qiskit Experiments to help with making the package more maintainable. Support for restless experiments is also deprecated with this change. Restless support is distinct from pulse support, but it is deprecated with the same motivation of simplifying the package overall. With improvements in the reliability of IBM Quantum's qubit initialization, circuit exectuion has already become reasonably fast and restless measurements do not add much performance improvement. It is expected that the restless features are little used as there has been no user feedback about them.
Also, add deprecation notes to the relevant manuals and tutorials and deprecation warnings to the relevant methods of BackendData and Backendtiming.
0501765
to
cec404c
Compare
Rebasing on main after being based on the branch of #1470 seems to have confused GitHub. |
Closing in favor of #1476 which GitHub is happy with. |
This change deprecates the experiments that rely on scanning the
parameters of pulses in pulse gate calibrations. Qiskit 2.0 will remove
support for pulse gate calibrations, making these experiments impossible
to run. The
Calibrations
andBasisGateLibrary
classes are alsodeprecated since they have no use without pulse gate calibrations to
track. It is planned that Qiskit Pulse will be moved to Qiskit Dynamics
and perhaps the experiments and calibrations can be adapted to that use
case for calibrating simulated experiments. For now though, this code is
removed from Qiskit Experiments to help with making the package more
maintainable. Related analysis classes and helper functions also
deprecated.
Support for restless experiments is also deprecated with this change.
Restless support is distinct from pulse support, but it is deprecated
with the same motivation of simplifying the package overall. With
improvements in the reliability of IBM Quantum's qubit initialization,
circuit exectuion has already become reasonably fast and restless
measurements do not add much performance improvement. It is expected
that the restless features are little used as there has been no user
feedback about them.