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] ExportedProgram #102259
[export] ExportedProgram #102259
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/102259
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d57b238: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
Thanks for the rapid pr! This looks good to me.
[ghstack-poisoned]
ghstack-source-id: b78ea1d863bea74f7e9a3cb11bdd3824b057cc9e Pull Request resolved: #102259
cc voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx [ghstack-poisoned]
cc voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx [ghstack-poisoned]
cc voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx [ghstack-poisoned]
cc voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx [ghstack-poisoned]
torch/_export/__init__.py
Outdated
input_name_to_example_inputs[node.name] = example_input | ||
input_tracker += 1 | ||
|
||
exported_program._input_shape_constraints = input_shape_constraints_by_src_name |
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.
Rebase over #102256
You should be able to grab input_shape_constraints_by_name from https://github.com/pytorch/pytorch/pull/102256/files#diff-cc590ca123eee83ede51a1c290198e331b1e528ea3d616f85cfcfe14afe03122R267
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 a fan of keeping these private fields. We should be able to eliminate ep._input_shape_constraints
in lieu of symbol_to_constraints
(generalizing symbol_to_range
to include equality constraints, which are basically other symbols).
Also, instead of _input_name_to_example_inputs
we should be able to just remember a map from names to the shapes of the example inputs, or even a map from (name, dim) pairs to constant values. Because the only reason we need them is for specialization assertions.
If you still want to keep the example inputs for additional debugging, then sure, keep them...but there should not be any remaining client for it after this change. I believe the right thing to do is to just forget them.
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.
Chatted with Avik offline -- I'll clean this up in a separate diff.
self.current_gm: Optional[torch.fx.GraphModule] = None | ||
self.constraints = self._process_shape_constraints(input_shape_constraints) | ||
self.input_name_to_example_inputs = input_name_to_example_inputs | ||
self.inline_constraints = inline_constraints | ||
|
||
def _process_shape_constraints(self, constraints) -> Dict[str, List[ConstraintSpec]]: |
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.
We shouldn't need this function if you move it before export, right?
torch/_export/passes/add_runtime_assertions_for_constraints_pass.py
Outdated
Show resolved
Hide resolved
if expr in self.inline_constraints: | ||
constraint = self.inline_constraints[expr] | ||
lower = _convert_to_int(constraint.lower) | ||
upper = _convert_to_int(constraint.upper) | ||
lower = _convert_to_int(constraint[0]) |
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.
We can move this before export by preprocessing input_name_to_example_inputs
cc voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx [ghstack-poisoned]
ghstack-source-id: fee3ffcbf73a576e6a3bfbbc4210eb2202e3b399 Pull Request resolved: #102259
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.
Thanks for working on this! This is beginning to shape up to something awesome (no pun intended lol).
Stack from ghstack (oldest at bottom):
cc @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx