-
Notifications
You must be signed in to change notification settings - Fork 105
[thunderfx] Integrate trace_structured
and trace_structured_artifact
into ThunderCompiler
to use TORCH_TRACE
and tlparse
#2182
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?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
84fb0db
to
05108cc
Compare
05108cc
to
6d8b13c
Compare
"encoding": "string", | ||
}, | ||
payload_fn=lambda: f"{trace_to_store}\n", | ||
compile_id=compile_options.get("torch_compile_compile_id", None), |
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 implicit dependency on compile_options
should be improved.
thunder/dynamo/compiler.py
Outdated
@@ -114,8 +127,29 @@ def __call__(self, gm: torch.fx.GraphModule, sample_args: list[torch.SymInt, tor | |||
|
|||
# The whole graph may not be supported by `thunder`, so we split it in `thunder` supported sections | |||
# and unsupported sections which are passed to `torch.compile(backend='inductor')` | |||
split_module, subgraph_info = _splitter(gm, self._thunder_jit, self._torch_compile, sample_args) | |||
split_module, subgraph_info = _splitter(gm, wrapped_thunder_jit, self._torch_compile, sample_args) |
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.
ditto
This gist might be helpful https://gist.github.com/crcrpar/cfdc7dbb499cbe6b327f04be5856f078 about |
trace_structured
and trace_structured_artifact
into ThunderCompiler
trace_structured
and trace_structured_artifact
into ThunderCompiler
to use TORCH_TRACE
and tlparse
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.
Pull Request Overview
This PR integrates structured tracing functionality from PyTorch's torch._logging._internal
into Thunder's compiler and splitter components to enable better debugging and analysis through tlparse
. The changes add trace artifacts at key compilation and splitting stages to provide visibility into Thunder's internal operations.
Key changes:
- Added structured tracing to capture graph modules at various stages (original, post-checkpoint conversion, split graphs)
- Added tracing for split reasons and unsupported nodes/contexts
- Integrated Thunder execution traces (computation, prologue, epilogue) with torch compile ID tracking
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
thunder/dynamo/splitter.py | Adds trace artifacts for unsupported nodes/contexts and original/processed graph modules during splitting |
thunder/dynamo/compiler.py | Adds tracing for original and split graphs, split reasons, and torch compile ID integration |
thunder/init.py | Adds structured tracing for Thunder execution traces with compile ID support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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 would be nice to have an example of end-to-end workflow in the PR description of how to use it with tlparse. It would be helpful for those (like me) who are not familiar with it.
@@ -428,6 +438,22 @@ def acquire_initial_trace(fn, args, kwargs, cd, cs, ad_hoc_executor): | |||
last_interpreter_log = jit_results.interpreter_log | |||
cs.last_interpreter_log = last_interpreter_log | |||
cs.last_interpreted_instructions = (i for i in last_interpreter_log if isinstance(i, dis.Instruction)) | |||
|
|||
for name_in_artifact, trace_to_store in ( |
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 think we should only invoke this logging function (_trace_structured
) if logging is specified by the user. This will also prevent failures on main path if this internal API changes.
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 agree. _log_to_torch_trace
could be _maybe_log_to_torch_trace
which incorporates the check on the user specifications.
Another thought:
def _log_to_torch_trace(
string_format:str,
trace_tuples: list[tuple],
compile_id,
):
for trc, *format_args in trace_tuples:
if trc is None:
continue
name = string_format.format(*format_args)
helper(name, trc, compile_id)
where helper
is _log_to_torch_trace
as defined below would allow these lines to be replaced with
trace_tuples = [
(computation_trc, "computation"), ...
]
_log_to_torch_trace(
"thunder_module_initial_{}_trc",
trace_tuples,
compile_id,
)
Maybe this isn't so much shorter, but it is less indentation. But up to you @crcrpar, whichever you find more readable.
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'm not that inclined to update the helper to take a list of tuples of trace and name. That indentation feels fine to me
thunder/__init__.py
Outdated
@@ -524,7 +550,23 @@ def apply_transforms_and_build_cache_entry(cd, cs, cache_info, prologue_trc, com | |||
if requires_grad: | |||
from thunder.transforms.autodiff import grad_transform_on_trace | |||
|
|||
_trace_structured( |
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 would be nice to have a helper function log_thunder_trace
(or something like that). It would be easier to read and also to use.
thunder/dynamo/splitter.py
Outdated
@@ -183,9 +207,30 @@ def is_thunder_supported_partition(node: torch.fx.Node) -> bool: | |||
partial(_get_example_inputs_from_placeholder, only_metadata=True), placeholders | |||
) | |||
example_input_metadatas.append(list(example_input_metadata)) | |||
|
|||
trace_structured_artifact( |
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 would be nice to have a helper called log_fx_graph
for readability and usage.
trace_structured
and trace_structured_artifact
into ThunderCompiler
to use TORCH_TRACE
and tlparse
trace_structured
and trace_structured_artifact
into ThunderCompiler
to use TORCH_TRACE
and tlparse
Oops, I noticed the current implementation only saves the last set of execution traces of a GraphModule. So if a graph module is chunked into thunder_0 -> inductor_1 -> thunder_2, then only the traces for thunder_2 seem to be saved. |
2bd6c36
to
ec7fb0c
Compare
@kshitij12345 @kiya00 @beverlylytle @riccardofelluga @IvanYashchuk any of you wanting to review this? (or someone else) |
so that we can use `TORCH_TRACE` and tlparse even for thunderfx Reference: - https://docs.pytorch.org/docs/stable/torch.compiler_troubleshooting.html#tlparse-torch-trace - [torch._logging._internal.trace_structured](https://github.com/pytorch/pytorch/blob/6f7694f18f6cbfc684fb44cc2f4f3a06218d919e/torch/_logging/_internal.py#L1209) - [torch._logging._internal.trace_structured_artifact](https://github.com/pytorch/pytorch/blob/6f7694f18f6cbfc684fb44cc2f4f3a06218d919e/torch/_logging/_internal.py#L1194) Signed-off-by: Masaki Kozuki <[email protected]>
as the former would be more Python-like. Signed-off-by: Masaki Kozuki <[email protected]>
…`tlparse` Signed-off-by: Masaki Kozuki <[email protected]>
also, propagating `CompileID` of torch.compile from `ThunderCompiler.__call__` to `thunder.jit` as compile id does not seem to be available when saving those artifacts Signed-off-by: Masaki Kozuki <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Co-authored-by: Copilot <[email protected]>
for more information, see https://pre-commit.ci
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
Signed-off-by: Masaki Kozuki <[email protected]>
acf6999
to
b48e9fe
Compare
What does this PR do?
This enables ThunderFX to save traces such as execution computation and backward traces as artifacts of
TORCH_TRACE
.By running scripts with
TORCH_TRACE=/path/to/torch_trace_log_dir
, without any changes in the scripts, we can check the traces as well as TorchDynamo outputtorch.fx.GraphModule
s and Inductor output, if fallback path is used.This PR inserts
trace_structured
andtrace_structured_artifact
into thunderfx optimization path.Ref:
In the following capture of an example of
TORCH_TRACE
, thunder_module_execution_computation_trc_12.txt has the execution forward trace.Example: