Deprecate Pulse package and dependencies

eliarbel commented 2 months ago

Summary

This PR deprecates the Pulse package and any non-pulse package code within Qiskit that uses it.

Closes #13063

Details and comments

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11468453533

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

416 of 440 (94.55%) changed or added relevant lines in 74 files are covered.
39 unchanged lines in 6 files lost coverage.
Overall coverage increased (+0.1%) to 88.67%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/dagcircuit/dagdependency.py	9	10	90.0%
qiskit/transpiler/passes/scheduling/base_scheduler.py	1	2	50.0%
qiskit/pulse/library/symbolic_pulses.py	20	24	83.33%
qiskit/transpiler/passes/calibration/rzx_builder.py	26	31	83.87%
qiskit/transpiler/passes/scheduling/dynamical_decoupling.py	2	7	28.57%
crates/circuit/src/dag_circuit.rs	43	51	84.31%
<!--	Total:	416	440	94.55%	-->

Files with Coverage Reduction	New Missed Lines	%
qiskit/transpiler/passes/scheduling/dynamical_decoupling.py	1	78.46%
qiskit/transpiler/target.py	1	94.05%
crates/accelerate/src/two_qubit_decompose.rs	1	92.09%
crates/circuit/src/imports.rs	2	77.78%
crates/qasm2/src/lex.rs	4	92.48%
crates/qasm2/src/parse.rs	30	97.15%
<!--	Total:	39	-->

Totals
Change from base Build 11411236768:	0.1%
Covered Lines:	74896
Relevant Lines:	84466

💛 - Coveralls

eliarbel commented 1 month ago

I think this PR is ready for review now as it contains all the deprecations, unit-test assertions, warning filtering and release notes I thought we should have for deprecating the pulse package. A couple notes worth highlighting and getting feedback on:

I've introduced a few deprecation decorators, specifically for pulse, located in qiskit.utils.deprecate_pulse.py. The aim of these decorators is to reuse deprecations messages and overall reduce code clutter due to the deprecation code in many places.
For unit test classes which interact heavily with pulse (e.g. all test cases in test/python/pulse) I used a new class level decorator which is used to suppress all deprecation warnings issued by pulse classes and methods. Same goes for calibration builder passes.
warnings.catch_warnings is used in several places which are called in common workflows such as transpile and working with Target. The motivation being is to prevent pulse-related deprecation from being emitted in cases where the user code does not call deprecated pulse APIs directly.

qiskit-bot commented 1 month ago

One or more of the following people are relevant to this code:

@enavarro51
@Qiskit/terra-core
@ajavadia
@levbishop
@mtreinish
@nkanazawa1989
@t-imamichi

eliarbel commented 1 month ago

Here is the result of a performance comparison of Qiskit benchmarks, showing only the change w.r.t the default ASV factor. There isn't a clear trend and I'm curious about what these changes mean really. Anyway for now I think we should wait a bit until the PR is stable and then rerun this and decide whether or not it's worthwhile to dive into this in more detail.

Change	Before [54fe09b2]	After [f78a9574]	Ratio	Benchmark (Parameter)
-	138±0.7μs	106±0.5μs	0.77	circuit_construction.CircuitConstructionBench.time_circuit_copy(14, 8192)
-	1.68±0.07ms	1.05±0.04ms	0.63	circuit_construction.CliffordSynthesis.time_clifford_synthesis(10)
+	1.02±0.02ms	1.69±0.2ms	1.65	mapping_passes.PassBenchmarks.time_dense_layout(14, 1024)
+	3.62±0ms	3.99±0.03ms	1.1	mapping_passes.PassBenchmarks.time_layout_2q_distance(14, 1024)
+	1.15±0.01ms	1.27±0ms	1.1	mapping_passes.PassBenchmarks.time_layout_2q_distance(5, 1024)
+	471±1ms	525±0.5ms	1.12	passes.MultipleBasisPassBenchmarks.time_basis_translator(14, 1024, ['rx', 'ry', 'rz', 'r', 'rxx', 'id'])
+	667±3ms	748±2ms	1.12	passes.MultipleBasisPassBenchmarks.time_basis_translator(20, 1024, ['rx', 'ry', 'rz', 'r', 'rxx', 'id'])
+	158±0.4ms	174±0.1ms	1.1	passes.MultipleBasisPassBenchmarks.time_basis_translator(5, 1024, ['rx', 'ry', 'rz', 'r', 'rxx', 'id'])
+	312±2ms	344±6ms	1.1	quantum_info.RandomCliffordBench.time_random_clifford('5,1000')
-	16.3±0.4ms	5.60±0.4ms	0.34	quantum_volume.QuantumVolumeBenchmark.time_ibmq_backend_transpile(1, 'translator')
-	18.1±1ms	6.35±0.05ms	0.35	transpiler_benchmarks.TranspilerBenchSuite.time_single_gate_compile
+	54.9±0.9ms	77.6±6ms	1.41	transpiler_qualitative.TranspilerQualitativeBench.time_transpile_time_cnt3_5_180(0, 'sabre', 'sabre')

ElePT commented 1 month ago

And regarding the benchmarks, I see what you mean, and in my experience the variability of the asv results is quite high, which makes certain improvements difficult to track (I have seen this with Unitary Synthesis, where the tests that didn't run it would also change quite a lot when benchmarking the PR).

eliarbel commented 1 month ago

The only missing point I think could be improved is my previous comment on adding a link to Qiskit Dynamics for the deprecated pulse dependencies.

Thanks @ElePT . I have implemented your suggestion already but haven't pushed it yet since I'm waiting for input from @nkanazawa1989 and @DanPuzzuoli regarding the exact dependencies which should be moved to Dynamics. Putting the list here so we can discuss in a wider forum if needed:

qiskit.compiler.schedule() qiskit.compiler.sequence() qiskit.scheduler.schedule_circuit() qiskit.scheduler.sequence() qiskit.scheduler.ScheduleConfig qiskit.scheduler.methods.as_soon_as_possible() qiskit.scheduler.methods.as_late_as_possible() qiskit.visualization.pulse_v2.draw()

Qiskit / qiskit