Skip to content

Add symbolic Gaussian Square Echo to Qiskit Pulse symbolic_pulses library#8757

Closed
miamico wants to merge 0 commit into
Qiskit:mainfrom
miamico:main
Closed

Add symbolic Gaussian Square Echo to Qiskit Pulse symbolic_pulses library#8757
miamico wants to merge 0 commit into
Qiskit:mainfrom
miamico:main

Conversation

@miamico
Copy link
Copy Markdown
Contributor

@miamico miamico commented Sep 14, 2022

Summary

Adding the implementation of a symbolic Gaussian Square Echo pulse

Details and comments

This is a first implementation of a symbolic version of Gaussian square echo pulse. Tests and documentation to be added. I have left the definition of the gap between echo pulses as it was but we may want to look at it more closely. Usually, the two echo pulses are separated by the duration of a single qubit gate which allows to echo the control qubit.

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Sep 14, 2022
@qiskit-bot
Copy link
Copy Markdown
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:

  • @Qiskit/terra-core

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 14, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
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.

Comment thread qiskit/pulse/library/symbolic_pulses.py Outdated
Comment thread qiskit/pulse/library/symbolic_pulses.py Outdated

class GaussianSquareEcho(SymbolicPulse):
"""An echoed Gaussian square pulse.
Exactly one of the ``risefall_sigma_ratio`` and ``width`` parameters has to be specified.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need description of the pulse shape (mathematical, or in some sentence) and it would be great if you have some reference paper. Perhaps user may want to know how this differs from GaussianSquare and when they can use this pulse.

Since this description about rise fall parameter is written in GaussianSquare, you can use .. seealso directive to document link to that class and avoid duplication.

Comment thread qiskit/pulse/library/symbolic_pulses.py Outdated
Comment thread qiskit/pulse/library/symbolic_pulses.py Outdated
@nkanazawa1989 nkanazawa1989 added the mod: pulse Related to the Pulse module label Sep 15, 2022
@miamico
Copy link
Copy Markdown
Contributor Author

miamico commented Sep 15, 2022

I'm glad to see we have other pulse shapes in Qiskit. If this is going to be backend-supported, you may also want to update

qobj converter,

https://github.com/Qiskit/qiskit-terra/blob/f2befd10b1aba6d9369daf4465cffdf75c7da576/qiskit/qobj/converters/pulse_instruction.py#L36-L46

and qpy loader

https://github.com/Qiskit/qiskit-terra/blob/f2befd10b1aba6d9369daf4465cffdf75c7da576/qiskit/qpy/binary_io/schedules.py#L94-L106

I think I have made the changes to those modules to support the GaussianSquareEcho pulse. I have a question though. I see that all the pulses have a corresponding equivalent in the continuous and discrete modules. Should I implement a gaussian square echo pulse there as well?

@nkanazawa1989
Copy link
Copy Markdown
Contributor

Implementation of waveform counterpart, i.e. continuous and discrete, is not mandatory, because we can get the same array with SymbolicPulse.get_waveform method (but nothing stops you to implement).

@miamico
Copy link
Copy Markdown
Contributor Author

miamico commented Jan 10, 2023

Picking this up again since this PR was merged recently. As it is now, GaussianSquareEcho only contains the Gaussian square pulses without the single qubit gates for the echo. There is a delay between the Gaussian Square pulses with positive and negative amplitudes where the second single qubit gate for the echo would fit. I wonder if we should include these single qubit gates or not. If not, how would we add the single qubit gate needed for the echo in the delay between the Gaussian squares?

@miamico miamico closed this Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: pulse Related to the Pulse module

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants