Skip to content

simplify tracing#623

Merged
sdpython merged 24 commits into
mainfrom
tracing
Feb 25, 2026
Merged

simplify tracing#623
sdpython merged 24 commits into
mainfrom
tracing

Conversation

@xadupre

@xadupre xadupre commented Feb 24, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread _unittests/ut_torch_interpreter/test_tracing.py

Copilot AI commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

@xadupre I've opened a new pull request, #624, to work on those changes. Once the pull request is ready, I'll request review from you.

xadupre and others added 3 commits February 24, 2026 13:54
…g in `CustomTracer` (#624)

* Initial plan

* Fix is_leaf_module to properly check module_leaves and clean up docstring

Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: xadupre <22452781+xadupre@users.noreply.github.com>

Copilot AI commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

@xadupre I've opened a new pull request, #625, to work on those changes. Once the pull request is ready, I'll request review from you.

@xadupre xadupre marked this pull request as ready for review February 24, 2026 13:33
Copilot AI review requested due to automatic review settings February 24, 2026 13:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the tracing/export pipeline in torch_interpreter to simplify pytree unflattening, add configurable FX leaf-module behavior during tracing, and extend “preserve submodules” support toward module-name-based selection.

Changes:

  • Simplifies tree_unflatten_with_proxy and expands CustomTracer with a module_leaves mechanism via is_leaf_module.
  • Adjusts to_onnx handling of export_modules_as_functions to only unflatten when the graph is an ExportedProgram.
  • Extends preserved-module selection to accept either module classes or qualified-name strings; adds/updates tracing/export unit tests.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
experimental_experiment/torch_interpreter/tracing.py Adds configurable leaf-module logic to FX tracing; simplifies pytree unflattening.
experimental_experiment/torch_interpreter/onnx_export.py Tweaks unflatten behavior under export_modules_as_functions and logging/stats around it.
experimental_experiment/torch_interpreter/interpreter.py Allows preserved-modules selection by type or module-qualified-name string.
experimental_experiment/torch_interpreter/export_options.py Wires tracer leaf-module config into tracing export path; changes GraphModule construction.
_unittests/ut_torch_interpreter/test_tracing.py Adds coverage for tracing/export behavior with preserved submodules and module leaves.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread experimental_experiment/torch_interpreter/interpreter.py
Comment thread experimental_experiment/torch_interpreter/export_options.py Outdated
Comment thread experimental_experiment/torch_interpreter/export_options.py Outdated
Comment thread _unittests/ut_torch_interpreter/test_tracing.py Outdated
Comment thread experimental_experiment/torch_interpreter/tracing.py Outdated
Comment thread _unittests/ut_torch_interpreter/test_tracing.py
Comment thread experimental_experiment/torch_interpreter/tracing.py Outdated
Comment thread experimental_experiment/torch_interpreter/interpreter.py
Comment thread experimental_experiment/torch_interpreter/onnx_export.py Outdated
Comment thread experimental_experiment/torch_interpreter/onnx_export.py Outdated
sdpython and others added 14 commits February 24, 2026 14:38
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@xadupre xadupre requested a review from Copilot February 24, 2026 22:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 126 to 130
def register_named_modules(
self,
parent_interpreter: Optional["DynamoInterpreter"],
preserved_modules: Optional[Set[type["torch.nn.Module"]]], # noqa: F821
preserved_modules: Optional[Set[Union[type["torch.nn.Module"], str]]], # noqa: F821
named_modules: Dict[str, "torch.nn.Module"], # noqa: F821

Copilot AI Feb 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

register_named_modules now allows preserved_modules entries to be str, but the verbose logging inside this method formats items using c.__name__, which will raise AttributeError for strings when builder.verbose > 4. Update the log formatting to handle both type and str (e.g., use the same (c if isinstance(c, str) else c.__name__) approach used in to_onnx).

Copilot uses AI. Check for mistakes.
Comment thread experimental_experiment/torch_interpreter/interpreter.py
Comment thread experimental_experiment/torch_interpreter/export_options.py Outdated
xadupre and others added 4 commits February 25, 2026 01:06
@sdpython sdpython merged commit 4b3886c into main Feb 25, 2026
7 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants