-
Notifications
You must be signed in to change notification settings - Fork 47
Add support for debugging PennyLane (Python) and Catalyst (C++) simultaneously #1712
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
e28ffbe
52d848b
6101c6f
4d1ff94
21686dd
dfd5007
97cd18b
44fbf63
e6b4849
a56a45c
6d77976
afba2ce
deb0502
23a8cc5
a6e7514
fcc3895
f40a06b
9c84fcb
dbe0908
25583b3
fa50232
004ebff
6ddad2a
634f8fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"version": "0.2.0", | ||
"configurations": [ | ||
{ | ||
"name": "(debugpy): Debug Current File", | ||
"type": "debugpy", | ||
"request": "launch", | ||
"program": "${file}", | ||
"console": "integratedTerminal", | ||
"justMyCode": false | ||
}, | ||
{ | ||
"name": "(gdb): Attach To Python Process", | ||
"type": "cppdbg", | ||
"request": "attach", | ||
"program": "${command:python.interpreterPath}", | ||
"processId": "${command:pickProcess}", | ||
"MIMode": "gdb", | ||
"setupCommands": [ | ||
{ | ||
"description": "Enable pretty-printing for gdb", | ||
"text": "-enable-pretty-printing", | ||
"ignoreFailures": true, | ||
} | ||
] | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,13 +21,15 @@ | |
import pathlib | ||
import platform | ||
import shutil | ||
import signal | ||
import subprocess | ||
import sys | ||
import tempfile | ||
import warnings | ||
from os import path | ||
from typing import List, Optional | ||
|
||
from catalyst.debug.debugger import is_debugger_active | ||
from catalyst.logging import debug_logger, debug_logger_init | ||
from catalyst.pipelines import CompileOptions | ||
from catalyst.utils.exceptions import CompileError | ||
|
@@ -438,15 +440,31 @@ def run_from_ir(self, ir: str, module_name: str, workspace: Directory): | |
output_ir_name = os.path.join(str(workspace), f"{module_name}.ll") | ||
|
||
cmd = self.get_cli_command(tmp_infile_name, output_ir_name, module_name, workspace) | ||
|
||
try: | ||
if self.options.verbose: | ||
print(f"[SYSTEM] {' '.join(cmd)}", file=self.options.logfile) | ||
result = subprocess.run(cmd, check=True, capture_output=True, text=True) | ||
|
||
with subprocess.Popen( | ||
cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True | ||
) as p: | ||
if p.returncode not in {0, None}: | ||
raise subprocess.CalledProcessError(p.returncode, cmd) | ||
|
||
if self.options.debug_compiler and is_debugger_active(): | ||
mlxd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
print(f"Compiler PID={p.pid}") | ||
print( | ||
f"""Ensure C++ debugger is attached and running before continuing with: | ||
kill -s SIGCONT {p.pid}""" | ||
) | ||
p.send_signal(signal.SIGSTOP) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know the process hasn't done any work yet by the time we get here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a good question. The assumption (from most places I've read) is that this should be (reasonably) fine. But if we 100% need to be strict on it, we can modify the process launch options to ensure the signal is the first thing that is hit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it just based on hoping that the OS doesn't schedule the subprocess thread until we hit this statement, or is there some other reason it would hold? For instance, does the process only start running once we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So For a quick 'n' dirty example of whether it is fast enough to ensure the process isn't running anything, we can opt for something akin to https://stackoverflow.com/questions/50002804/create-subprocess-in-python-in-suspended-state In practice, use of the import os
import signal
def f():
"pre-execute function to print the new process PID"
print(os.getpid())
print("Starting process")
p = Popen(["/tmp/test"], preexec_fn=f)
print("Running process")
# Stop process after creating the Process and a print call.
# If the `/tmp/test` binary can output before this line is hit,
# we need to stop within the preexec_fn. Otherwise, all good
# to set up the process.
p.send_signal(signal.SIGSTOP)
# Have some wait-time that we control
i = input("enter/return to continue")
# Allow direct input/output from communicate
p.communicate()
# It should be done by now
print("Finished process")
# explicitly kill the process
p.kill()
print("Killed process") Reasoning for favouring not relying on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the info! I'll leave it up to you then if we want to use preexec or not, although in the current approach maybe we could at least send the signal before printing? 😅
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine by me. We can always revert if the above CPython changes make it to a release. Update: As an aside, since the printing will happen from the subprocess, we may have issues with capturing the stdout and stderr for reporting. I'll see if this is something that can be mitigated, but if not, we just use the existing. |
||
|
||
res_stdout, res_stderr = p.communicate() | ||
if self.options.verbose or os.getenv("ENABLE_DIAGNOSTICS"): | ||
if result.stdout: | ||
print(result.stdout.strip(), file=self.options.logfile) | ||
if result.stderr: | ||
print(result.stderr.strip(), file=self.options.logfile) | ||
if res_stdout: | ||
print(res_stdout.strip(), file=self.options.logfile) | ||
if res_stderr: | ||
print(res_stderr.strip(), file=self.options.logfile) | ||
except subprocess.CalledProcessError as e: # pragma: nocover | ||
raise CompileError(f"catalyst failed with error code {e.returncode}: {e.stderr}") from e | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
# Copyright 2025 Xanadu Quantum Technologies Inc. | ||
|
||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
|
||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
""" | ||
This module adds functionality to check if the active Python session | ||
is being run with an active debugger. | ||
""" | ||
|
||
|
||
import sys | ||
|
||
|
||
def is_debugger_active() -> bool: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this function useful, if we don't use it internally? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at this function again, I don't think we should add it to the Catalyst API, it's completely unrelated to Catalyst code. |
||
"""Will return true in active debugger session""" | ||
return hasattr(sys, "gettrace") and sys.gettrace() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,6 +94,7 @@ def qjit( | |
circuit_transform_pipeline=None, | ||
pass_plugins=None, | ||
dialect_plugins=None, | ||
debug_compiler=False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I would have preferred not adding another keyword to this decorator, but it's not the end of the world 😌 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, i'd agree it was nice -- maybe we can chat next week and see which seems better. This way allows attaching from a non-VSCode/non-Python debug session, so if people prefer to use it without, they can. But happy to defer either way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I was able to get it running now and the process is pretty straightforward, so I'm happy to go this route if you prefer :) Although, I just thought of another alternative. If we want to contain this sort of functionality to the debug module, we could add a context manager for emitting the signal, similar to the instrumentation one. A bit more inline with what we already have, and keeps it out of the regular user options. Just an idea though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, not a bad idea. Let me see what is needed to try that approach. If it becomes a pain, we can always support that in a follow-up, and leave this as usable in the current form for now. |
||
): # pylint: disable=too-many-arguments,unused-argument | ||
"""A just-in-time decorator for PennyLane and JAX programs using Catalyst. | ||
|
||
|
@@ -161,6 +162,8 @@ def qjit( | |
If not specified, the default pass pipeline will be applied. | ||
pass_plugins (Optional[List[Path]]): List of paths to pass plugins. | ||
dialect_plugins (Optional[List[Path]]): List of paths to dialect plugins. | ||
debug_compiler (Optional[bool]): Enable external debugger attachment to the compiler | ||
driver when launching from an active Python debugging environment. | ||
|
||
Returns: | ||
QJIT object. | ||
|
Uh oh!
There was an error while loading. Please reload this page.