-
Notifications
You must be signed in to change notification settings - Fork 586
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
Adding documentation for qml.breakpoint()
and qml.PLDB
#5789
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Jaybsoni, left some minor corrections. Please make sure that
- We are consistent with using small/capital letters in Pdb and PLDB.
- The links to PL pages, e.g., QNode, are working.
Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! We'll add a code example to the changelog when we rewrite it for the release 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Jaybsoni! I know this was just merged, but I have a few more comments that I'd like to be addressed.
This function marks a location in a quantum circuit (QNode). When it is encountered during | ||
execution of the quantum circuit, an interactive debugging prompt is launched to step | ||
throught the circuit execution using Pdb like commands (:code:`list`, :code:`next`, | ||
:code:`continue`, :code:`quit`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we link to a central page that lists the commands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a link to the qml.debugging
api page.
|
||
.. code-block:: python3 | ||
|
||
dev = qml.device("default.qubit", wires=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev = qml.device("default.qubit", wires=2) | |
dev = qml.device("default.qubit") |
It's fine to include, but we can skip the wires
argument since switching to the new device API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes for sure, this is because when we call qml.debug_state()
on a circuit while in execution, we display all of the device wires. If we remove the wires argument, then it will only display those wires which the circuit has encountered so far in the execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove it from this example as its not required for it?
|
||
circuit(1.23) | ||
|
||
Running the above python script opens up the interactive :code:`[pldb]:` prompt in the terminal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outside of PL debugging, is it normally [pdb]:
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes its a different prompt, normally Its (Pdb)
def state(): | ||
"""Compute the state of the quantum circuit. | ||
def debug_state(): | ||
"""Compute the quantum state at the current point in the quantum circuit. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth mentioning that this does not alter the state in the rest of the circuit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for expval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to the documentation for state
, expval
and probs
.
:code:`continue`, :code:`quit`). | ||
|
||
**Example** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth mentioning the debug_x
functions we've added? E.g., with a note/see also box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add a see also link to the debugging module documentation, the example there covers the rest of the functions?
OR, if you prefer I can list all of the debug_x functions and link to their individual documentation pages.
@@ -132,33 +132,198 @@ def pldb_device_manager(device): | |||
|
|||
|
|||
def breakpoint(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can we have it so that breakpoint
and debug_x
all show up as top level in the documentation? Currently they are shown as within the debugging
module (e.g., here), which is true but not the location we're recommending users access from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They appear both in the top level documentation and the module documentation. Let me try and remove the module documentation links.
.. automodapi:: pennylane.debugging | ||
:no-heading: | ||
:no-inherited-members: | ||
:skip: PLDB | ||
:skip: pldb_device_manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intended for snapshots to show up here? 🤔
Overall, it might make sense. Just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though snapshots
is also a top-level accessible function but it's showing up as qml.debugging.snapshots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was intended on purpose to place snapshots here. Originally, the snapshots logic and the debugger were both located in the same python file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to this module, it would be nice to have a short section just below here in the quickstart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 👍🏼
`qml.debug_expval()`, and `qml.debug_tape()`). Consider the following python script | ||
containing the quantum circuit with breakpoints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a typo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, the code example was removed but this transition sentence was not, I can remove it.
**Context:** This is a follow up to this [PR](#5789) which added detailed documentation for the new PennyLane Debugger feature. --------- Co-authored-by: Astral Cai <astral.cai@xanadu.ai> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Yushao Chen (Jerry) <chenys13@outlook.com> Co-authored-by: Christina Lee <chrissie.c.l@gmail.com> Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
**Context:** Adding documentation for the new debugger feature. --------- Co-authored-by: Mikhail Andrenkov <mikhail@xanadu.ai> Co-authored-by: Utkarsh <utkarshazad98@gmail.com> Co-authored-by: Diego <67476785+DSGuala@users.noreply.github.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com> Co-authored-by: Diego <diego_guala@hotmail.com>
**Context:** This is a follow up to this [PR](#5789) which added detailed documentation for the new PennyLane Debugger feature. --------- Co-authored-by: Astral Cai <astral.cai@xanadu.ai> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Yushao Chen (Jerry) <chenys13@outlook.com> Co-authored-by: Christina Lee <chrissie.c.l@gmail.com> Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Context:
Adding documentation for the new debugger feature.