-
Notifications
You must be signed in to change notification settings - Fork 24.5k
Process inputs and outputs in fx interpreter #74242
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
Conversation
CI Flow Status⚛️ CI FlowRuleset - Version:
|
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit ec8af03 (more details on the Dr. CI page):
🕵️ 15 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
This pull request was exported from Phabricator. Differential Revision: D34898108 |
This pull request was exported from Phabricator. Differential Revision: D34898108 |
012418f
to
eb154f3
Compare
Summary: Pull Request resolved: pytorch#74242 The inputs and outputs of the graph module might be different from the graph inputs and outputs if users are using custom codegen. In interpreter, it runs the graph instead of the generated forward function so it might not work if user provides the inputs to the graph module. To fill the gap, we call `process_inputs` and `process_outputs` inside interpreter. Test Plan: unit test: test_interpreter_with_codegen Reviewed By: Chillee Differential Revision: D34898108 fbshipit-source-id: 447c2817504b415e4c4fe269434b10ee64ea87ca
eb154f3
to
ec8af03
Compare
This pull request was exported from Phabricator. Differential Revision: D34898108 |
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.
Seems fine to me, but @Chillee should probably take a look as well
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.
BC test is fine, since you've added a kwarg with default it should be BC (however there is a BC breaking vector where someone overrides that method, but I don't think we have any mitigation possible for it). Feel free to --accept
the change
Summary: Pull Request resolved: #74242 The inputs and outputs of the graph module might be different from the graph inputs and outputs if users are using custom codegen. In interpreter, it runs the graph instead of the generated forward function so it might not work if user provides the inputs to the graph module. To fill the gap, we call `process_inputs` and `process_outputs` inside interpreter. Test Plan: unit test: test_interpreter_with_codegen Reviewed By: jamesr66a, Chillee Differential Revision: D34898108 fbshipit-source-id: 250bd236f6c8c1268a363cf19a09521a4f64b3a9
Hey @842974287. |
Looks like this broke the BC tests on master (didn't run |
@pytorchbot revert this |
Reverting PR 74242 failed due to 'nodyText' |
cc @malfet for error, reverting in phab |
This pull request has been reverted by bf5e25f. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk). |
Summary: Pull Request resolved: pytorch#74637 Forgot to update the expect file in pytorch#74242. Reland to include changes in expect file. Test Plan: unit test Differential Revision: D35089989 fbshipit-source-id: 7697274eff5333d218d522ef105c33b9f1e1ba70
Summary: Pull Request resolved: #74242 The inputs and outputs of the graph module might be different from the graph inputs and outputs if users are using custom codegen. In interpreter, it runs the graph instead of the generated forward function so it might not work if user provides the inputs to the graph module. To fill the gap, we call `process_inputs` and `process_outputs` inside interpreter. Test Plan: unit test: test_interpreter_with_codegen Reviewed By: jamesr66a, Chillee Differential Revision: D34898108 fbshipit-source-id: 250bd236f6c8c1268a363cf19a09521a4f64b3a9 (cherry picked from commit b33076f)
This pull request has been reverted by bf5e25f. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk). |
Summary: The inputs and outputs of the graph module might be different from the graph inputs and outputs if users are using custom codegen. In interpreter, it runs the graph instead of the generated forward function so it might not work if user provides the inputs to the graph module. To fill the gap, we call
process_inputs
andprocess_outputs
inside interpreter.Test Plan: unit test: test_interpreter_with_codegen
Differential Revision: D34898108