Refactor ExportedProgram to expose the functions for pre and postprocessing #119513
Conversation
qihqi
requested review from
avikchaudhuri,
gmagogsfm,
tugsbayasgalan and
zhxchen17
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/119513
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 657bba0 with merge base cd9a193 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
discussed previously with @zhxchen17 |
torch/export/exported_program.py
Outdated
There was a problem hiding this comment.
@angelayi is working on removing this API. we can leave it as-is in the diff.
There was a problem hiding this comment.
Removing __call__?
What is the replacement?
There was a problem hiding this comment.
@qihqi we're changing to ep.module()(*args, **kwargs), because in a lot of cases people want a drop in replacement torch.nn.Module, and .module() does that.
There was a problem hiding this comment.
why not keep __call__ and it forwards to self.module()(...)?
There was a problem hiding this comment.
that's also okayish to do, but we just feel it might cause some confusion to people, because there're two ways to call the same thing from API perspective.
halfjoking, that's not very pythonic(tm)
|
More context: introduction of Thanks! |
janeyx99
added
the
triaged
qihqi
force-pushed
the
fix_export1
branch
2 times, most recently
from
7bd5995 to
426e004
Compare
torch/export/exported_program.py
Outdated
There was a problem hiding this comment.
@qihqi since you mentioned you want to land it soon, could we make temporarily make it a private API (_graph_module_flat_inputs)? In that way you can be unblocked and we can have some time later to make a formal design discussion.
There was a problem hiding this comment.
@zhxchen17 done. Please keep me in the loop when these functions get renamed to become public.
|
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot label "topic: not user facing" |
|
@pytorchbot merge |
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 |
…essing (#119513) Reason: Consumers of ExportProgram might choose to further lower exported_program.graph_module to something else. Then, it will need to setup the calling convention to call it. This refactor concentrates these calling convention to one place and can be reused. Fixes #ISSUE_NUMBER Pull Request resolved: #119513 Approved by: https://github.com/zhxchen17
5 participants
Reason:
Consumers of ExportProgram might choose to further lower exported_program.graph_module
to something else.
Then, it will need to setup the calling convention to call it.
This refactor concentrates these calling convention to one place and can be reused.
Fixes #ISSUE_NUMBER