Implement `qml.breakpoint()` and initial integration with `Pdb` #5600

Jaybsoni · 2024-04-30T14:57:24Z

Context:
Part of the effort to add more tools for algorithmic debugging of quantum circuits with PennyLane. In this PR we introduce the qml.breakpoint() function which allows users to insert breakpoints into their quantum workflow and enter a debugging context (via Pdb) to step through their quantum functions.

Description of the Change:

Added a PLDB class which inherits most of its functionality from Pdb.
Added functionality to pass active device information into the debugger during qnode execution to validate the compatibility between the device and the debugger.

github-actions · 2024-04-30T15:09:03Z

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

A one-to-two sentence description of the change. You may include a small working example for new features.
A link back to this PR.
Your name (or GitHub username) in the contributors section.

Jaybsoni · 2024-04-30T15:09:39Z

[sc-62243]

Jaybsoni · 2024-04-30T17:45:47Z

Open Questions:

Do we want to take any specialized actions when users call step down or step up? Restrict it? Raise a warning? (Currently it is allowed and leads the user into complex pennylane internals)
How should we test the breakpoint functionality without triggering the interactive debugger in the test suite? Are there any industry standards here?
I have currently added validation checks for incompatible devices (anything other than lightning.qubit and default.qubit) as well is if the breakpoint is called outside of a qnode execution. Any thoughts on this?

mlxd · 2024-04-30T17:56:12Z

pennylane/debugging.py

+        Args:
+            dev (Union[Device, "qml.devices.Device"]): The active device
+        """
+        cls.__active_dev.append(dev)


So devices are add-only, not not selectively removed --- as in, we expect them to be appended, and reset the entire list to remove?

Yes exactly! In fact, instead of appending, we could probably just override the 1st entry in the list each time, since there shouldn't be more than one "active device" at a time.

Yea, if only 1 device is a constraint, adapting to match that design sounds reasonable.

If we only need to support one device, it could be worth making the active device just a ClassVar. It's minimal syntax and simple to use. If, however, we think in the future it is likely that we'll need to support multiple devices, then defining these methods makes sense.

I don't think we will want multi-device support in the future,

pennylane/debugging.py

mlxd · 2024-04-30T18:01:53Z

pennylane/workflow/qnode.py

+        if PLDB.is_active_dev():
+            PLDB.reset_active_dev()
+
+        PLDB.add_device(self.device)


Just a quick check, but this setup shouldn't have any problem with concurrent execution, right? If we in parallel create 2 qnodes with 2 devices, what happens here if they try to modify the class at the same time?

They will most definitely clash! I definitely think this feature should not be used with concurrent execution, I don't know if a user would ever want to actually debug the circuit for 2 qnodes concurrently?

I based this implementation on how the Queuing manger is structured, how does that correspond with concurrent execution

Is there a way to implement this without a queuing manager approach?
If not, as a quick validation, can we try using the debugger with default.qubit and max_workers >= 2 to see what happens.

Tried this with max_workers=3 and it seemed to work normally (had to reduce OMP_NUM_THREADS). Not sure if it will still work for qml.debugging.state() when it is implemented.

codecov · 2024-04-30T18:14:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.67%. Comparing base (fbc2a39) to head (26f28f9).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5600      +/-   ##
==========================================
- Coverage   99.68%   99.67%   -0.01%     
==========================================
  Files         416      416              
  Lines       39105    38432     -673     
==========================================
- Hits        38981    38307     -674     
- Misses        124      125       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

trbromley

Thanks @Jaybsoni!

trbromley · 2024-05-01T15:26:04Z

pennylane/debugging.py

+class PLDB(pdb.Pdb):
+    """Custom debugging class integrated with Pdb."""


To confirm, this isn't something users would typically interact with?

Yes exactly! We don't want the user calling the PLDB class at all. In the same way we wouldn't want them to interact with the QueuingManager class for example.

trbromley · 2024-05-01T15:28:31Z

pennylane/debugging.py

+
+
+def breakpoint():
+    """Launch the custom PennyLane debugger."""


What is the plan with docs here - normally PRs should have detailed docs, especially for major user-facing functions like this. Though if we have a story to track a subsequent docs revisit and polish, I'm fine with that too.

There is a story to revisit the docs and polish. I will add a little more detail to the breakpoint docs in this PR, but once all of the functionality (measuring states, queuing gates in debug mode, etc.) are added in the subsequent PRs, then we can finalize its docstring.

Minor point that we can discuss in a docs PR:

We don't have a quickstart (.rst) for debugging yet. And it doesn't seem like we make quickstarts for any of the high level files in the pennylane folder. Maybe there's a question here about whether debugging functionality should go in pennylane/debugging.py or in pennylane/debugging/debugging.py

DSGuala

Looks good to me!

qml.breakpoint() pauses execution ✔️
Tested the commands while debugging (list, next, continue, quit) and they all worked fine. ✔️ One note here: hitting next at the return statement of the circuit puts us into PL internals, but I think that's fine.
Can access internal and external variables from the debugging context ✔️
Can queue additional operations ✔️

Pending for next PRs:

qml.debugging.state()
qml.debugging.expval()
qml.debugging.tape()

I think we can keep this open and merge other PRs into this branch. Having the docs will especially help with PR reviews.

DSGuala · 2024-05-23T20:42:55Z

pennylane/debugging.py

+
+        Raises:
+            TypeError: Can't call breakpoint outside of a qnode execution
+            TypeError: Device not supported with breakpoint


Maybe:

Suggested change

TypeError: Device not supported with breakpoint

TypeError: Breakpoints not supported on this device

DSGuala · 2024-05-23T20:43:21Z

pennylane/debugging.py

+            raise TypeError("Can't call breakpoint outside of a qnode execution")
+
+        if cls.get_active_device().name not in ("default.qubit", "lightning.qubit"):
+            raise TypeError("Device not supported with breakpoint")


Suggested change

raise TypeError("Device not supported with breakpoint")

raise TypeError("Breakpoints not supported on this device")

DSGuala · 2024-05-23T20:49:06Z

pennylane/workflow/qnode.py

+        if PLDB.is_active_dev():
+            PLDB.reset_active_dev()
+
+        PLDB.add_device(self.device)


Tried this with max_workers=3 and it seemed to work normally (had to reduce OMP_NUM_THREADS). Not sure if it will still work for qml.debugging.state() when it is implemented.

Mandrenkov

Neat stuff, @Jaybsoni! I left a few minor comments and suggestions.

Mandrenkov · 2024-05-24T13:59:09Z

pennylane/debugging.py

@@ -105,3 +108,75 @@ def get_snapshots(*args, **kwargs):
        return dbg.snapshots

    return get_snapshots
+
+
+class PLDB(pdb.Pdb):


(Satire) Missed opportunity to call this Qdb. 😆

We could still change it! Thoughts @trbromley

I defer to @DSGuala, but either way works for me! I don't think users will interact with this functionality so would be happy to stick with PLDB.

Mandrenkov · 2024-05-24T14:03:06Z

pennylane/debugging.py

+        if not qml.queuing.QueuingManager.recording() or not cls.is_active_dev():
+            raise TypeError("Can't call breakpoint outside of a qnode execution")
+
+        if cls.get_active_device().name not in ("default.qubit", "lightning.qubit"):


Question

Is this because there is something specific about the implementation of these devices? Or just because they have a certain interface? If it's the latter, we should define the interface using a Protocol decorated with runtime_checkable() and simply check if the device supports the interface here.

Mandrenkov · 2024-05-24T14:04:47Z

pennylane/debugging.py

+        """
+
+        if not qml.queuing.QueuingManager.recording() or not cls.is_active_dev():
+            raise TypeError("Can't call breakpoint outside of a qnode execution")


Nitpick

Is this really a type error? Or is it more of a RuntimeError?

Mandrenkov · 2024-05-24T14:07:54Z

pennylane/debugging.py

+            Union[Device, "qml.devices.Device"]: The active device
+        """
+        if not cls.is_active_dev():
+            raise ValueError("No active device to get")


Nitpick

Similarly, the issue is not that an argument to this method is incorrect:

Suggested change

raise ValueError("No active device to get")

raise RuntimeError("No active device to get")

Mandrenkov · 2024-05-24T14:09:35Z

pennylane/debugging.py

+        Args:
+            dev (Union[Device, "qml.devices.Device"]): The active device
+        """
+        cls.__active_dev.append(dev)


If we only need to support one device, it could be worth making the active device just a ClassVar. It's minimal syntax and simple to use. If, however, we think in the future it is likely that we'll need to support multiple devices, then defining these methods makes sense.

Mandrenkov · 2024-05-24T14:10:53Z

pennylane/debugging.py

+
+    def __init__(self, *args, **kwargs):
+        """Initialize the debugger, and set custom prompt string."""
+        super().__init__(*args, **kwargs)


Note

We can pass a skip argument to avoid stepping into PennyLane internals. From the docs,

The skip argument, if given, must be an iterable of glob-style module name patterns. The debugger will not step into frames that originate in a module that matches one of these patterns.

Mandrenkov · 2024-05-24T14:12:06Z

pennylane/workflow/qnode.py

+        # Before constructing the tape, we pass the device to the
+        # debugger to ensure they are compatible if there are any
+        # breakpoints in the circuit
+        if PLDB.is_active_dev():
+            PLDB.reset_active_dev()
+
+        PLDB.add_device(self.device)


Suggestion

This works, although it could potentially make sense to just have the "setting" of an active device as a context manager (so that the caller doesn't need to "remember" to reset the active device). This is similar to the approach JAX uses.

tests/test_debugging.py

soranjh · 2024-05-24T21:18:03Z

tests/test_debugging.py

+
+        PLDB.reset_active_dev()  # clean up the debugger active devices
+
+    dev_names = (


What prevents us from supporting other devices?

As long as its a simulator device, we should be able to support it, but for this MVP we are aimining to gurantee support for these two devices.

trbromley

No blockers to merging from me! Thanks @Jaybsoni!

trbromley · 2024-05-28T16:42:40Z

pennylane/debugging.py

@@ -105,3 +108,75 @@ def get_snapshots(*args, **kwargs):
        return dbg.snapshots

    return get_snapshots
+
+
+class PLDB(pdb.Pdb):


I defer to @DSGuala, but either way works for me! I don't think users will interact with this functionality so would be happy to stick with PLDB.

implement breakpoint()

20eef13

Jaybsoni added the WIP 🚧 Work-in-progress label Apr 30, 2024

Jaybsoni added 2 commits April 30, 2024 13:38

added docstring and tests

c814e7e

Merge branch 'master' into algo_debug_breakpoint

f120900

format

508bb8b

Jaybsoni requested review from soranjh, trbromley and DSGuala April 30, 2024 17:49

Jaybsoni changed the title ~~[WIP] Implement qml.breakpoint() and initial integration with Pdb~~ Implement qml.breakpoint() and initial integration with Pdb Apr 30, 2024

Jaybsoni added review-ready 👌 PRs which are ready for review by someone from the core team. and removed WIP 🚧 Work-in-progress labels Apr 30, 2024

mlxd reviewed Apr 30, 2024

View reviewed changes

trbromley reviewed May 1, 2024

View reviewed changes

Jaybsoni added 3 commits May 7, 2024 12:42

Merge branch 'master' into algo_debug_breakpoint

3d7de97

Add missing test coverage

18bffe2

Merge branch 'master' into algo_debug_breakpoint

f82ea4d

Jaybsoni requested review from mlxd and trbromley May 10, 2024 21:02

Jaybsoni added 3 commits May 21, 2024 16:30

Merge branch 'master' into algo_debug_breakpoint

86829ee

Merge branch 'master' into algo_debug_breakpoint

ad62173

Merge branch 'master' into algo_debug_breakpoint

f45dc3e

Jaybsoni requested a review from Mandrenkov May 23, 2024 20:15

DSGuala reviewed May 23, 2024

View reviewed changes

Mandrenkov reviewed May 24, 2024

View reviewed changes

soranjh reviewed May 24, 2024

View reviewed changes

Merge branch 'master' into algo_debug_breakpoint

26f28f9

trbromley reviewed May 28, 2024

View reviewed changes

		class PLDB(pdb.Pdb):
		"""Custom debugging class integrated with Pdb."""



		def breakpoint():
		"""Launch the custom PennyLane debugger."""

	TypeError: Device not supported with breakpoint
	TypeError: Breakpoints not supported on this device

	raise TypeError("Device not supported with breakpoint")
	raise TypeError("Breakpoints not supported on this device")

	raise ValueError("No active device to get")
	raise RuntimeError("No active device to get")


		PLDB.reset_active_dev() # clean up the debugger active devices

		dev_names = (

Implement qml.breakpoint() and initial integration with Pdb #5600

Are you sure you want to change the base?

Implement qml.breakpoint() and initial integration with Pdb #5600

Conversation

Jaybsoni commented Apr 30, 2024 • edited

github-actions bot commented Apr 30, 2024

Jaybsoni commented Apr 30, 2024

Jaybsoni commented Apr 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 30, 2024 • edited

Codecov Report

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DSGuala left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mandrenkov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Question

Choose a reason for hiding this comment

Nitpick

Choose a reason for hiding this comment

Nitpick

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Note

Choose a reason for hiding this comment

Suggestion

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement `qml.breakpoint()` and initial integration with `Pdb` #5600

Implement `qml.breakpoint()` and initial integration with `Pdb` #5600

Jaybsoni commented Apr 30, 2024 •

edited

codecov bot commented Apr 30, 2024 •

edited

DSGuala left a comment •

edited