-
Notifications
You must be signed in to change notification settings - Fork 106
[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
Open
crcrpar
wants to merge
25
commits into
main
Choose a base branch
from
logging-for-torch_trace-tlparse
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+177
−6
Open
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
2d618ec
integrate `trace_structured` & `trace_structured_artifact`
crcrpar c34db96
use `GraphModule.print_readable` not `str(GraphModule.graph)`
crcrpar 8f11a06
(ab)use `trace_structured_artifact` to make split reasons handled by …
crcrpar 04f64b8
Store execution prologue, computation, and epilogue traces
crcrpar 7441d67
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] a140a17
use `trace_structured` to specify `compile_id`
crcrpar ddced44
removing `trace_structured` as it requires `tlparse` customization
crcrpar 8e26859
remove trace_structured
crcrpar 032e109
clean up following `trace_structured` removal
crcrpar 2c98d8a
fix name of GraphModules inside splitter
crcrpar d3b5378
Apply suggestions from code review of copilot
crcrpar e6209e3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 5a9f1e5
deduplicate `metadata_fn`
crcrpar c2eb191
avoid redefinition of trace
crcrpar b722c0d
store backward_trc if available
crcrpar 23dd2bb
store trace before/after grad_transform
crcrpar 2482974
check `compile_id` arg
crcrpar 5280335
add todo comment
crcrpar 6e1689a
remove `compile_id` from `_trace_structured` callsite
crcrpar 2bb659e
fix
crcrpar c64b83e
[no ci] todo: use node index
crcrpar ac7b851
propagate chunk index of GraphModule
crcrpar aa87d1e
wrap trace_structured for trace/graph-module
crcrpar 59765a6
fix typos
crcrpar b48e9fe
clean up
crcrpar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
from collections import defaultdict, namedtuple | ||
from collections.abc import Callable, Sequence | ||
from contextvars import ContextVar | ||
from functools import wraps | ||
from functools import partial, wraps | ||
from typing import Any | ||
import dis | ||
import inspect | ||
import os | ||
import time | ||
import warnings | ||
|
@@ -56,6 +57,7 @@ | |
wrap_return_value_together_with_arguments, | ||
) | ||
from thunder.core.update_aliases import insert_alias_updates | ||
from thunder.dynamo._trace_structured import _log_to_torch_trace | ||
from thunder.executors.torch_autograd import connect_to_autograd | ||
import thunder.extend as extend | ||
from thunder.extend import Executor, add_default_executor | ||
|
@@ -436,6 +438,19 @@ 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 ( | ||
("computation", computation_trc), | ||
("prologue", prologue_trc), | ||
("epilogue", epilogue_trc), | ||
): | ||
if trace_to_store is None: | ||
continue | ||
_log_to_torch_trace( | ||
f"thunder_module_initial_{name_in_artifact}_trc", | ||
trace_to_store, | ||
compile_id=compile_options.get("torch_compile_compile_id", None), | ||
) | ||
return prologue_trc, computation_trc, epilogue_trc | ||
|
||
def apply_transforms_and_build_cache_entry(cd, cs, cache_info, prologue_trc, computation_trc, epilogue_trc): | ||
|
@@ -532,7 +547,17 @@ 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 | ||
|
||
_log_to_torch_trace( | ||
"thunder_module_computation_trc_before_grad_transform", | ||
computation_trc, | ||
compile_id=compile_options.get("torch_compile_compile_id", None), | ||
) | ||
computation_trc = grad_transform_on_trace(computation_trc) | ||
_log_to_torch_trace( | ||
"thunder_module_computation_trc_after_grad_transform", | ||
computation_trc, | ||
compile_id=compile_options.get("torch_compile_compile_id", None), | ||
) | ||
|
||
from thunder.executors.passes import _transform_for_operator_executor_execution | ||
from thunder.distributed.utils import maybe_sort_waits | ||
|
@@ -598,6 +623,22 @@ def apply_transforms_and_build_cache_entry(cd, cs, cache_info, prologue_trc, com | |
computation_trc = transform_to_torch_types(computation_trc) | ||
comp = computation_trc.python_callable() | ||
|
||
for name_in_artifact, trace_to_store in ( | ||
("computation", computation_trc), | ||
("prologue", prologue_trc), | ||
("epilogue", epilogue_trc), | ||
("backward", backward_trc), | ||
): | ||
if trace_to_store is None: | ||
continue | ||
|
||
_idx_of_graph_module = compile_options.get("graph_module_idx", 0) | ||
_log_to_torch_trace( | ||
f"thunder_module_execution_{name_in_artifact}_trc_of_module_{_idx_of_graph_module}", | ||
trace_to_store, | ||
compile_id=compile_options.get("torch_compile_compile_id", None), | ||
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. This implicit dependency on |
||
) | ||
|
||
# TODO RC1 Update the cache | ||
cache_entry = CacheEntry( | ||
pro, | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
from __future__ import annotations | ||
from typing import TYPE_CHECKING | ||
import inspect | ||
|
||
from torch.fx import GraphModule | ||
from torch._logging._internal import trace_structured | ||
from torch._logging._internal import trace_structured_artifact | ||
|
||
if TYPE_CHECKING: | ||
from collections.abc import Callable | ||
from typing import Any | ||
from torch._guards import CompileId | ||
|
||
|
||
_SUPPORT_COMPILE_ID_KWARG: bool = "compile_id" in inspect.signature(trace_structured).parameters | ||
|
||
|
||
def payload_fn_of(fn: GraphModule | Callable[[Any], Any]) -> Callable[[], str]: | ||
if isinstance(fn, GraphModule): | ||
|
||
def f() -> str: | ||
return fn.print_readable( | ||
print_output=False, | ||
include_stride=True, | ||
include_device=True, | ||
) | ||
|
||
return f | ||
|
||
def f() -> str: | ||
return f"{fn}\n" | ||
|
||
return f | ||
|
||
|
||
# TODO: use `trace_structured_artifact` once `compile_id` is merged. | ||
# https://github.com/pytorch/pytorch/pull/160440. | ||
# note: `compile_id` is a kwarg since v2.7.0. | ||
def _log_to_torch_trace( | ||
name: str, | ||
fn: GraphModule | Callable[[Any], Any], | ||
compile_id: CompileId | None = None, | ||
) -> None: | ||
payload_fn = payload_fn_of(fn) | ||
if compile_id is not None and _SUPPORT_COMPILE_ID_KWARG: | ||
trace_structured( | ||
"artifact", | ||
metadata_fn=lambda: { | ||
"name": name, | ||
"encoding": "string", | ||
}, | ||
payload_fn=payload_fn, | ||
) | ||
else: | ||
trace_structured_artifact( | ||
name=name, | ||
encoding="string", | ||
payload_fn=payload_fn, | ||
) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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:
where
helper
is_log_to_torch_trace
as defined below would allow these lines to be replaced withMaybe 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