Repository navigation
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/23691
Note: Links to docs will display an error until the docs builds have been completed.
|
Contributor
Author
|
@pytorchbot label "release notes: devtools" |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
The Inspector can't be built from an ETRecord of a multi-method program. Since #17185, ETRecord stores
edge_dialect_programas a dict of method name -> program when there's more than one method, butgen_graphs_from_etrecordand_resolve_reference_graphstill do.graph_moduleon it, soInspector(etdump_path=..., etrecord=...)fails right away withAttributeError: 'dict' object has no attribute 'graph_module'.This adds a small helper,
get_forward_edge_dialect_program, that returns the program as-is when it's a single one, or the "forward" entry when it's a dict, and uses it in those two places. The Inspector already only correlates the forward method, so that's the graph it should use.Programs with no method named "forward" still fail earlier in ETRecord handling (KeyError: 'forward'). That needs a bigger decision about which method to inspect, so I left it out of this one.
Test plan
Added test_inspector_with_multi_method_etrecord to devtools/inspector/tests/inspector_test.py. It exports a two-method program (forward + times_two) with generate_etrecord=True, builds an Inspector from the saved ETRecord, and checks the reference graph is the forward graph. It fails on main with the AttributeError above and passes with the fix, and the rest of devtools/inspector/tests passes. ufmt and flake8 are clean with the versions from requirements-lintrunner.txt.
cc @nil-is-all