-
Notifications
You must be signed in to change notification settings - Fork 527
Add pass to remove unused parameters in to_executorch #10484
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
base: main
Are you sure you want to change the base?
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/10484
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 3087578 with merge base 7e034ca ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This pull request was exported from Phabricator. Differential Revision: D73654202 |
Summary: Currently, ExecuTorch will serialize any parameters in the exported program, regardless of whether they are actually used. Exporting with strict=True will remove unused parameters, but strict=False will not. Export recently switched to non-strict as the default behavior. This causes PTE bloat when doing pt2e quantization (unquantized weights are left in the graph) or sometimes when exporting multiple methods (encode and decoder, for example). This PR adds a new pass (`remove_unused_parameters_pass`) to strip unused parameters from the `ExportedProgram`. It is run as part of `to_executorch`. Parameters are considered unused if there are no uses of the placeholder node. Parameters are removed by stripping them from the state_dict, input specs, and graph. As a question for reviewers, should we run this pass earlier, as part of to_edge? My rationale for running as part of to_executorch was that it could theoretically clean up anything else left by partitioning and lowering, but I'm not aware of any concrete use cases for this. Differential Revision: D73654202
c518552
to
3087578
Compare
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Can we run it in to_edge instead? We should remove unused things as soon as possible imo. |
Sure, I'll move it to there. |
) -> bool: | ||
placeholder_node = placeholder_nodes.get(parameter) | ||
if placeholder_node is None: | ||
# Shouldn't happen, but in this case, leave the parameter to be safe. |
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.
throw an exception?
Summary:
Currently, ExecuTorch will serialize any parameters in the exported program, regardless of whether they are actually used. Exporting with strict=True will remove unused parameters, but strict=False will not. Export recently switched to non-strict as the default behavior.
This causes PTE bloat when doing pt2e quantization (unquantized weights are left in the graph) or sometimes when exporting multiple methods (encode and decoder, for example).
This PR adds a new pass (
remove_unused_parameters_pass
) to strip unused parameters from theExportedProgram
. It is run as part ofto_executorch
. Parameters are considered unused if there are no uses of the placeholder node. Parameters are removed by stripping them from the state_dict, input specs, and graph.As a question for reviewers, should we run this pass earlier, as part of to_edge? My rationale for running as part of to_executorch was that it could theoretically clean up anything else left by partitioning and lowering, but I'm not aware of any concrete use cases for this.
Differential Revision: D73654202
cc @JacobSzwejbka @angelayi