Converting the pulse library from complex amp to amp+angle

[TsafrirA]

Summary

We convert the symbolic pulses in the pulse library from a complex amp to real (float) amp+angle. i.e., instead of calling (for example):
Gaussian(duration=25, sigma=4, amp=0.5j),
one should call:
Gaussian(duration=25, sigma=4, amp=0.5, angle = np.pi/2).
Backward compatibility is maintained, and deprecation warnings are raised when the previous API is used.

Details and comments

The API is changed, and now expects two float parameters for the magnitude of the amplitude and the angle. During the deprecation period, complex amp will be handled as well, with a deprecation warning. Note that even when a complex amplitude is provided, the _params dictionary will include an additional angle entry with value 0. It should be emphasized that the roles described here for parameters amp and angle are dictated only for library pulses. These roles are not assumed for custom built pulses.

Because IBM backends expect a complex amp for the library pulses, one has to include a casting to complex variables. Previously, this was done throughout the code in various points associated with the creation of pulses. Now, the casting is shifted only to the conversion to qobj. In the conversion, the amp entry is converted to complex(amp*exp(1j*angle)), and the angle entry is deleted.

The qpy loader was adjusted to load pulses created with older versions in a way which will conform with the new API. To do so (without bumping QPY version), the qiskit version with which the QPY file was created is used to differentiate old and new APIs (note that this required passing qiskit_version to many of the functions of the loading process). Loading of library pulses of versions 0.22.2 or lower is then done by adding a 0 zero valued angle to parameters, because complex amp is still supported. In future releases, once complex amp support is deprecated, this will have to be adjusted again to convert those pulses to (amp,angle) representation.

It should be noted that no warning is raised when amp is assigned a complex parameter via a Parameter class object. This was done to avoid an assumption of a general role for "amp" across all pulses (and due to the difficulty of differentiating library pulses from custom made pulses).

Tests were added to make sure warnings and errors are raised in the appropriate scenarios, and that the pulses created with both API choices are the same. Older tests were updated to the new API, or removed when no longer necessary (particularly tests guaranteeing the casting to complex).

Because negative amp is allowed, pulses are no longer unique. Future PR will address this issue by equating the pulses with a consideration of their envelopes' expressions.

Close #6097

Many thanks to @nkanazawa1989 and @mtreinish for their help.

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

• @Qiskit/terra-core
• @nkanazawa1989

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[nkanazawa1989]

Thanks @TsafrirA ! This is awesome start of the important refactoring. There is very old issue #6097 regarding this, and I really wanted to address. This is important change to align Qiskit Experiments calibrations with the typical representation in the standard production system.

Could you please also update QPY load? You can check amplitude type and convert it into a float (amp, angle) pair when complex.
https://github.com/Qiskit/qiskit-terra/blob/be1e6976138201cefa021e55dd2f0dac99bf6a1f/qiskit/qpy/binary_io/schedules.py#L85-L90

The PulseQobj loader would suffer the same problem. It's hard to update existing production systems, and the backends will continue to report calibrations in complex amplitude. So you need to manage the input format otherwise user will always see annoying deprecation warnings when one load a backend.
https://github.com/Qiskit/qiskit-terra/blob/be1e6976138201cefa021e55dd2f0dac99bf6a1f/qiskit/qobj/converters/pulse_instruction.py#L727-L730

In addition, we need to consider the parameter assignment logic here.
https://github.com/Qiskit/qiskit-terra/blob/be1e6976138201cefa021e55dd2f0dac99bf6a1f/qiskit/pulse/parameter_manager.py#L233-L239

I prefer to use (amp, angle) in repr. This is much readable.

Close #6097

[nkanazawa1989]

Thanks @TsafrirA. I think this is almost good to go. I prefer to allow negative amplitude with finite angle for the convenience of parameter sweep in calibrations, but we need to address equality issues in the separate PR, i.e. (amp, pi) and (-amp, 0) must be equal. Can you also add release note and remove WIP?

[nkanazawa1989]

Thanks @TsafrirA for updating the PR. I think now backward compatibility is managed properly and the qobj converters and qpy module are updated correctly. Namely,

• A complex value amp only appears in the library pulse (gaussian, gaussian_square, drag, constant) with terra <= 0.22 (or in qpy version <=5). Thus qpy pulse data generated with old terra must be converted into new format in newer terra (or in qpy version 6+).
• We must remove the special casing (role) of amp from the Qiskit user space and move it to the qobj converter. This is a convention in the transport layer. If a user still want complex amplitude one can use a custom build symbolic pulse. Since qpy data is typed, complex parameter is qpy round-tripped properly.

I just added few more minor suggestions.

[nkanazawa1989]

Thanks @TsafrirA now this PR looks good to me. After you fix the merge conflict on test/python/pulse/test_calibrationbuilder.py, I'll trigger CI and merge this PR.

[nkanazawa1989]

Thanks @TsafrirA the update to QPY module looks good to me. Just a nitpick.

[nkanazawa1989]

This is great work! Indeed it was challenging work to properly guarantee the backward compatibility for both qpy and qobj converter as well as library pulses. Thanks for finding many edge cases and resolving them. Finally this looks good to go.