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

Added pulses to the SymbolicPulse library #9625

Merged
merged 12 commits into from
Feb 21, 2023
Merged

Conversation

TsafrirA
Copy link
Collaborator

@TsafrirA TsafrirA commented Feb 20, 2023

Summary

The old library of discrete pulses included several pulse types which were not present in the SymbolicPulse library. These pulses include Sin, Cos, Sawtooth and Triangle, which were added now to the SymbolicPulse library.

Details and comments

The SymbolicPulse library was extended to include 4 new pulse types:

  • Sin
  • Cos
  • Sawtooth
  • Triangle

With the exception of the Sawtooth phase parameter, the behavior is identical to the corresponding discrete pulses. The phase parameter of Sawtooth was modified such that a 2\pi phase shift will result in a full cycle shift, and not a \pi shift, as is the case in the discrete pulse sawtooth.

Following the example set by #9329, the pulses are introduced as functions, but their names are capitalized to match the old pulses.

The new pulses were not introduced to the qpy_compat test. To do that, the test would have to be split into several versions, according to the availability of different pulses in different releases. (Some versioning will be required anyway once the deprecation of complex amplitude will be completed - #9002)

@TsafrirA TsafrirA requested review from a team, eggerdj and wshanks as code owners February 20, 2023 12:32
@qiskit-bot
Copy link
Collaborator

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:

@coveralls
Copy link

coveralls commented Feb 20, 2023

Pull Request Test Coverage Report for Build 4235675563

  • 46 of 47 (97.87%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 85.288%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/pulse/library/symbolic_pulses.py 46 47 97.87%
Totals Coverage Status
Change from base Build 4232248032: 0.009%
Covered Lines: 67319
Relevant Lines: 78931

💛 - Coveralls

Copy link
Contributor

@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 @TsafrirA this looks almost in good shape. Glad to see the growth of pulse library :)

qiskit/pulse/library/symbolic_pulses.py Outdated Show resolved Hide resolved
qiskit/pulse/library/symbolic_pulses.py Outdated Show resolved Hide resolved
qiskit/pulse/library/symbolic_pulses.py Outdated Show resolved Hide resolved
test/python/pulse/test_pulse_lib.py Outdated Show resolved Hide resolved
TsafrirA and others added 2 commits February 21, 2023 12:41
Copy link
Contributor

@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 @TsafrirA this looks good to go.

@nkanazawa1989 nkanazawa1989 added mod: pulse Related to the Pulse module Changelog: New Feature Include in the "Added" section of the changelog automerge labels Feb 21, 2023
@mergify mergify bot merged commit 3512bbd into Qiskit:main Feb 21, 2023
Eric-Arellano pushed a commit to Eric-Arellano/qiskit-terra that referenced this pull request Feb 23, 2023
* Corrected library pulses return types.

* Added Sin and Cos Pulses

* Add sawtooth pulse

* Add triangle pulse

* Add new pulses to _init_ and some corrections

* Add tests, release notes and some corrections.

* Release notes correction

* Documentation correction

* Update qiskit/pulse/library/symbolic_pulses.py

Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>

* Some corrections and modifications

---------

Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@TsafrirA TsafrirA deleted the NewPulses branch June 25, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: pulse Related to the Pulse module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants