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
[export] Add run_decomposition() function to ExportedProgram #110236
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110236
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 1 PendingAs of commit bf37d65 with merge base bc047ec (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This pull request was exported from Phabricator. Differential Revision: D49743208 |
f692ec8
to
487e933
Compare
This pull request was exported from Phabricator. Differential Revision: D49743208 |
|
||
if len(new_range_constraints) > 0 or len(new_equality_constraints) > 0: | ||
exported_program = exported_program._transform( | ||
_AddRuntimeAssertionsForInlineConstraintsPass( |
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.
Why do we need this?
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.
I'm also not sure if this is 100% needed. If it's possible that some decomposed operator introduces an unbacked symint, then we will also need to add assertions for it 😅
new_range_constraints, new_equality_constraints | ||
) | ||
) | ||
exported_program = lift_constant_tensor_pass(exported_program) |
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.
So basically anytime we run aot_export, we need to re-run lift_constant_tensor_pass and _ReplaceSymSizeOpPass()?
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.
I'm not sure if this is 100% needed. Basically, if it's possible that some decomposed operator introduces a constant tensor, then we will need to lift it.
).run(core_aten_ep.graph_module.code) | ||
self.assertTrue(torch.allclose(core_aten_ep(*inp), m(*inp))) | ||
|
||
def test_export_decomps_dynamic(self): |
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.
I am all for having more tests. Just a bit curious why dynamic shape affects run-decomposition.
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.
I just wanted to check that the dynamic shapes were preserved through aot export.
487e933
to
2d68649
Compare
…#110236) Summary: https://docs.google.com/document/d/1QJJEGnj2nHGPODlw38BEG3KLLCOTfdOVjPrNQbz_LM8/edit#bookmark=id.lp80wfshq130 `exported_program.run_decompositions(decomposition_table)` will optionally take a decomposition table, and run decompositions on the exported program, returning a new exported program. By default we will run the Core ATen decomposition table. Splitting up this diff with the following one (D49742989) to make migrating Executorch easier: 1. Land this diff 1. Wait for a pytorch nightly to include this diff 1. Update executorch's pytorch nightly pin 1. Land the following diff to have export() return no decomps + updates executorch code to use the run_decomps() Test Plan: Tested in following diff Differential Revision: D49743208
This pull request was exported from Phabricator. Differential Revision: D49743208 |
…#110236) Summary: https://docs.google.com/document/d/1QJJEGnj2nHGPODlw38BEG3KLLCOTfdOVjPrNQbz_LM8/edit#bookmark=id.lp80wfshq130 `exported_program.run_decompositions(decomposition_table)` will optionally take a decomposition table, and run decompositions on the exported program, returning a new exported program. By default we will run the Core ATen decomposition table. Splitting up this diff with the following one (D49742989) to make migrating Executorch easier: 1. Land this diff 1. Wait for a pytorch nightly to include this diff 1. Update executorch's pytorch nightly pin 1. Land the following diff to have export() return no decomps + updates executorch code to use the run_decomps() Test Plan: Tested in following diff Differential Revision: D49743208
2d68649
to
e9b8f28
Compare
This pull request was exported from Phabricator. Differential Revision: D49743208 |
…#110236) Summary: https://docs.google.com/document/d/1QJJEGnj2nHGPODlw38BEG3KLLCOTfdOVjPrNQbz_LM8/edit#bookmark=id.lp80wfshq130 `exported_program.run_decompositions(decomposition_table)` will optionally take a decomposition table, and run decompositions on the exported program, returning a new exported program. By default we will run the Core ATen decomposition table. Splitting up this diff with the following one (D49742989) to make migrating Executorch easier: 1. Land this diff 1. Wait for a pytorch nightly to include this diff 1. Update executorch's pytorch nightly pin 1. Land the following diff to have export() return no decomps + updates executorch code to use the run_decomps() Test Plan: Tested in following diff Reviewed By: tugsbayasgalan Differential Revision: D49743208
e9b8f28
to
1509970
Compare
This pull request was exported from Phabricator. Differential Revision: D49743208 |
…#110236) Summary: https://docs.google.com/document/d/1QJJEGnj2nHGPODlw38BEG3KLLCOTfdOVjPrNQbz_LM8/edit#bookmark=id.lp80wfshq130 `exported_program.run_decompositions(decomposition_table)` will optionally take a decomposition table, and run decompositions on the exported program, returning a new exported program. By default we will run the Core ATen decomposition table. Splitting up this diff with the following one (D49742989) to make migrating Executorch easier: 1. Land this diff 1. Wait for a pytorch nightly to include this diff 1. Update executorch's pytorch nightly pin 1. Land the following diff to have export() return no decomps + updates executorch code to use the run_decomps() Test Plan: Tested in following diff Reviewed By: tugsbayasgalan Differential Revision: D49743208
1509970
to
bf37d65
Compare
This pull request was exported from Phabricator. Differential Revision: D49743208 |
@pytorchbot merge (Initiating merge automatically since Phabricator Diff has merged) |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Summary:
https://docs.google.com/document/d/1QJJEGnj2nHGPODlw38BEG3KLLCOTfdOVjPrNQbz_LM8/edit#bookmark=id.lp80wfshq130
exported_program.run_decompositions(decomposition_table)
will optionally take a decomposition table, and run decompositions on the exported program, returning a new exported program. By default we will run the Core ATen decomposition table.Splitting up this diff with the following one (D49742989) to make migrating Executorch easier:
Test Plan: Tested in following diff
Differential Revision: D49743208