Conversation
…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>
There was a problem hiding this comment.
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_proxyand expandsCustomTracerwith amodule_leavesmechanism viais_leaf_module. - Adjusts
to_onnxhandling ofexport_modules_as_functionsto only unflatten when the graph is anExportedProgram. - 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.
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>
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.