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

Deprecate qpy loading of old library pulses with complex amp #11257

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

TsafrirA
Copy link
Collaborator

Summary

This PR completes #10357 and adds a deprecation warning when loading an old QPY file containing library symbolic pulses with complex amp.

Details and comments

The deprecation process began in #9002, but #10357 neglected to promote this specific warning to a deprecation warning. The PR is meant for 0.46, while 1.0.0 will have the support removed.

@TsafrirA TsafrirA marked this pull request as ready for review November 16, 2023 06:03
@TsafrirA TsafrirA requested a review from a team as a code owner November 16, 2023 06:03
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @mtreinish
  • @nkanazawa1989

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM! I take that the next step will be the removal PR against main. Thanks for following through with the slightly annoying deprecation procedure :)

Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

I just realized that the stacklevel of the deprecation warning is not set (it defaults to 1), which means that the message will not be seen unless a user explicitly calls _read_symbolic_pulse (which is unlikely). The level should match the number of function calls that happen between qpy.load and _read_symbolic_pulse, and this is actually kind of tricky in this case, because it's not a fixed number. I will bring it up to the team and think of a solution.

@ElePT ElePT added this to the 0.46.0 milestone Nov 21, 2023
@nkanazawa1989 nkanazawa1989 added mod: pulse Related to the Pulse module mod: qpy Related to QPY serialization labels Nov 27, 2023
@ElePT
Copy link
Contributor

ElePT commented Dec 20, 2023

Even though #11260 isn't merged yet, I will go ahead and add this PR to the merge queue, and take care of changing the warning class of loading complex amp. as part of #11260. This will also let us double check that the warning is raised properly as part of the PR that introduces the new warning class, which I think makes sense.

@ElePT ElePT added this pull request to the merge queue Dec 20, 2023
Merged via the queue into Qiskit:stable/0.46 with commit 6ff0b93 Dec 20, 2023
13 checks passed
@TsafrirA TsafrirA deleted the ComplexAmp0_46 branch February 8, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: pulse Related to the Pulse module mod: qpy Related to QPY serialization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants