Skip to content

Support multi-module exports in Inspector and ETRecord #8336

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
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cptspacemanspiff
Copy link
Contributor

@cptspacemanspiff cptspacemanspiff commented Feb 10, 2025

So, this is a prototype of changes required to get performance data out of a multi-method export with ETRecords and ETDump.

I do not have a good understanding of if this is correct/the best way, and my understanding of the performance analysis stuff is limited.

What this does:

  1. In _etrecord.py, it allows for having a blank edge_dialect_program, and just exporting the ETRecord with export_modules:
export_modules [Optional]: **Should be ignored by OSS users**. A dictionary of graph modules with the key being the user provided name and the
            value being the corresponding exported module. The exported graph modules can be either the
            output of `torch.export()` or `exir.to_edge()`.
  1. In the inspector tool, adds the ability to specify the module/method to pull the data from the ETRecord (since we are not using the global edge_program_method.)

FWIW I was able to get the statistics table with node labeling with these changes.

From the sounds of this this might be used internally and there is probably a better way to handle this?

related to #8030

- Added optional module_name and method_name parameters to Inspector initialization
- Updated _consume_etrecord method to handle multi-module export scenarios
- Modified gen_graphs_from_etrecord to support custom graph keys (for each module/method)
- Updated inspector_cli to accept module and method name arguments
- Improved error handling for multi-module export use cases
Copy link

pytorch-bot bot commented Feb 10, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/8336

Note: Links to docs will display an error until the docs builds have been completed.

❌ 5 New Failures, 2 Cancelled Jobs

As of commit becb19a with merge base d99970b (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 10, 2025
@cptspacemanspiff
Copy link
Contributor Author

@JacobSzwejbka

@jackzhxng jackzhxng added the release notes: devtools Changes to dev tooling, for example the debugger & profiler label Feb 12, 2025
@shoumikhin
Copy link
Contributor

@JacobSzwejbka, @tarun292, @Gasoonjia can you take a look please?

raise RuntimeError(
f"Unsupported type of edge_dialect_program passed in {type(edge_dialect_program)}."
)
if export_modules is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure i fully understand why you needed to add this check here?

"--module_name",
required=False,
default=None,
help="Module Name to inspect (used with multi-module exports)",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is module name intended to be here?

@byjlw
Copy link
Contributor

byjlw commented Mar 31, 2025

@cptspacemanspiff any update on this? I'm seeing a couple open questions to the code still

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: devtools Changes to dev tooling, for example the debugger & profiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants