Skip to content

Docathon! Docs for discrete pulse functions #3838

Merged
mergify[bot] merged 16 commits into
Qiskit:masterfrom
DanPuzzuoli:pulse-lib-doc-2
Feb 14, 2020
Merged

Docathon! Docs for discrete pulse functions #3838
mergify[bot] merged 16 commits into
Qiskit:masterfrom
DanPuzzuoli:pulse-lib-doc-2

Conversation

@DanPuzzuoli
Copy link
Copy Markdown
Contributor

@DanPuzzuoli DanPuzzuoli commented Feb 13, 2020

Closes #3392

Summary

Adding math expressions for the functions being sampled in discrete.py.

Details and comments

  1. Updated doc strings in discrete.py to include math expressions, along with a couple of jupyter-execute sections for plotting pulses with a simple shape but whose mathematical expression may not be immediately recognizable (or at least weren't to me).
  2. Code provided by @SooluThomas: Updated pulse.rst and pulse/__init__.py to include a section in the sphinx pulse toc called Pulse Library, which links to the discrete module.

@DanPuzzuoli DanPuzzuoli changed the title [WIP] Docathon! Docs for discrete pulse functions Docathon! Docs for discrete pulse functions Feb 13, 2020
Copy link
Copy Markdown
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

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

Looks great, just some small changes recommended.

Comment thread qiskit/pulse/pulse_lib/discrete.py Outdated
Comment thread qiskit/pulse/pulse_lib/discrete.py Outdated
Comment thread qiskit/pulse/pulse_lib/discrete.py Outdated
SooluThomas
SooluThomas previously approved these changes Feb 14, 2020
@SooluThomas
Copy link
Copy Markdown
Member

Thank you @DanPuzzuoli

Copy link
Copy Markdown
Contributor

@lcapelluto lcapelluto left a comment

Choose a reason for hiding this comment

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

This is wonderful!! Looks so good now when rendered.

I had a minor question about a factor of two, pretty sure we include it. Here is what I mean, from continuous:

def gaussian(times: np.ndarray, amp: complex, center: float, sigma: float,
             zeroed_width: Optional[float] = None, rescale_amp: bool = False,
             ret_x: bool = False) -> Union[np.ndarray, Tuple[np.ndarray, np.ndarray]]:
    times = np.asarray(times, dtype=np.complex_)
    x = (times-center)/sigma
    gauss = amp*np.exp(-x**2/2).astype(np.complex_)

Comment thread qiskit/pulse/__init__.py Outdated
Comment thread qiskit/pulse/pulse_lib/discrete.py Outdated
Comment thread qiskit/pulse/pulse_lib/discrete.py Outdated
@lcapelluto lcapelluto self-assigned this Feb 14, 2020
@lcapelluto lcapelluto added the documentation Something is not clear or an error documentation label Feb 14, 2020
DanPuzzuoli and others added 3 commits February 14, 2020 13:21
Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>
Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>
Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>
@DanPuzzuoli
Copy link
Copy Markdown
Contributor Author

Nice catch @lcapelluto, just committed

@lcapelluto lcapelluto dismissed taalexander’s stale review February 14, 2020 20:06

suggestions were applied

@mergify mergify Bot merged commit 2c6693d into Qiskit:master Feb 14, 2020
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
* adding math functions to doc strings in discrete.py, and messing with sphinx table of contents

* further work on function documentation

* completed another pass of discrete.py fixing sphinx issues

* Updated pulse.rst and pulse/__init__.py to include docs correctly as per @SooluThomas' solution

* reshuffling sphinx toc stuff

* de-indented the second two lines in citation to avoid the 'header' style appearance in the docs. Will need to revisit this once handling of citations is sorted

* Update qiskit/pulse/pulse_lib/discrete.py

Co-Authored-By: Thomas Alexander <thomasalexander2718@gmail.com>

* Update qiskit/pulse/pulse_lib/discrete.py

Co-Authored-By: Thomas Alexander <thomasalexander2718@gmail.com>

* Update qiskit/pulse/pulse_lib/discrete.py

Co-Authored-By: SooluThomas <soolu.elto@gmail.com>

* re-adding doc string to pulse_lib/__init__.py

* fixed one more error in the drag function- mathematical description did not include beta

* Update qiskit/pulse/__init__.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/pulse/pulse_lib/discrete.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/pulse/pulse_lib/discrete.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

Co-authored-by: Thomas Alexander <thomasalexander2718@gmail.com>
Co-authored-by: SooluThomas <soolu.elto@gmail.com>
Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Something is not clear or an error documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add mathematical definition of pulses in pulse_lib documentation

4 participants