[Quant][fx] Refactor lowering code (part 1) #74128
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
CI Flow Status⚛️ CI FlowRuleset - Version:
|
Contributor
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 149bb03 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
jerryzh168
approved these changes
Mar 15, 2022
Contributor
jerryzh168
left a comment
jerryzh168
left a comment
There was a problem hiding this comment.
thanks, looks good! ideally we should separate them to multiple PRs I think, but maybe you can try to do that next time
Contributor
|
maybe also add some more details for the refactors in the summary? |
Summary: This commit removes extraneous and duplicate code in _lower_to_native_backend.py. There should be no change in behavior. Test Plan: python test/test_quantization.py TestQuantizeFx Reviewers: jerryzh168 Subscribers: jerryzh168 [ghstack-poisoned]
Contributor
Author
|
Ok, how about I just do the removing subgraph rewriter and |
andrewor14
changed the title
andrewor14
added a commit
that referenced
this pull request
Mar 15, 2022
Summary: This commit is the first step towards refactoring the lowering code in _lower_to_native_backend.py. The main changes included in this commit are: (1) Remove the use of the subgraph rewriter in lowering (2) Replace the use of `is_match` with manual pattern matching The motivation behind (2) is it simplifies the lowering code significantly; previously we had many different but similar patterns for slightly different models. There should be no change in behavior with this PR. Note that this is only part 1 of the refactoring. Part 2 will merge the static and dynamic lowering code paths and refactor the currently duplicate pattern matching / cleanup code into common helper functions. Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168 Subscribers: jerryzh168 ghstack-source-id: de7bbb7 Pull Request resolved: #74128
**Summary:** This commit is the first step towards refactoring the lowering code in _lower_to_native_backend.py. The main changes included in this commit are: (1) Remove the use of the subgraph rewriter in lowering (2) Replace the use of `is_match` with manual pattern matching The motivation behind (2) is it simplifies the lowering code significantly; previously we had many different but similar patterns for slightly different models. There should be no change in behavior with this PR. Note that this is only part 1 of the refactoring. Part 2 will merge the static and dynamic lowering code paths and refactor the currently duplicate pattern matching / cleanup code into common helper functions. **Test Plan:** python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps **Reviewers:** jerryzh168 **Subscribers:** jerryzh168 [ghstack-poisoned]
andrewor14
added a commit
that referenced
this pull request
Mar 15, 2022
Summary: This commit is the first step towards refactoring the lowering code in _lower_to_native_backend.py. The main changes included in this commit are: (1) Remove the use of the subgraph rewriter in lowering (2) Replace the use of `is_match` with manual pattern matching The motivation behind (2) is it simplifies the lowering code significantly; previously we had many different but similar patterns for slightly different models. There should be no change in behavior with this PR. Note that this is only part 1 of the refactoring. Part 2 will merge the static and dynamic lowering code paths and refactor the currently duplicate pattern matching / cleanup code into common helper functions. Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168 Subscribers: jerryzh168 ghstack-source-id: 630d882 Pull Request resolved: #74128
Contributor
Author
|
@andrewor14 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Contributor
Author
|
Ok, I updated the PR description with more details about the refactor. Provided that the tests pass this PR is ready from my side |
andrewor14
added a commit
that referenced
this pull request
Mar 16, 2022
Summary: This commit is the first step towards refactoring the lowering code in _lower_to_native_backend.py. The main changes included in this commit are: (1) Remove the use of the subgraph rewriter in lowering (2) Replace the use of `is_match` with manual pattern matching The motivation behind (2) is it simplifies the lowering code significantly; previously we had many different but similar patterns for slightly different models. There should be no change in behavior with this PR. Note that this is only part 1 of the refactoring. Part 2 will merge the static and dynamic lowering code paths and refactor the currently duplicate pattern matching / cleanup code into common helper functions. Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168 Subscribers: jerryzh168 ghstack-source-id: 630d882 Pull Request resolved: #74128
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 17, 2022
Summary: Pull Request resolved: #74128 **Summary:** This commit is the first step towards refactoring the lowering code in _lower_to_native_backend.py. The main changes included in this commit are: (1) Remove the use of the subgraph rewriter in lowering (2) Replace the use of `is_match` with manual pattern matching The motivation behind (2) is it simplifies the lowering code significantly; previously we had many different but similar patterns for slightly different models. There should be no change in behavior with this PR. Note that this is only part 1 of the refactoring. Part 2 will merge the static and dynamic lowering code paths and refactor the currently duplicate pattern matching / cleanup code into common helper functions. **Test Plan:** python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps **Reviewers:** jerryzh168 **Subscribers:** jerryzh168 Test Plan: Imported from OSS Reviewed By: jerryzh168 Differential Revision: D34910597 Pulled By: andrewor14 fbshipit-source-id: c6fea0c538ce5efc5afaf53e072922528988dda7
Contributor
|
Hey @andrewor14. |
andrewor14
added
release notes: quantization
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Summary: This commit is the first step towards refactoring the
lowering code in _lower_to_native_backend.py. The main changes
included in this commit are:
(1) Remove the use of the subgraph rewriter in lowering
(2) Replace the use of
is_matchwith manual pattern matchingThe motivation behind (2) is it simplifies the lowering code
significantly; previously we had many different but similar
patterns for slightly different models. There should be no
change in behavior with this PR.
Note that this is only part 1 of the refactoring. Part 2
will merge the static and dynamic lowering code paths
and refactor the currently duplicate pattern matching /
cleanup code into common helper functions.
Test Plan:
python test/test_quantization.py TestQuantizeFx
python test/test_quantization.py TestQuantizeFxOps
Reviewers: jerryzh168
Subscribers: jerryzh168
Differential Revision: D34910597