-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
[Inductor] [Quant] Re-structure Quantization testcase pattern matcher check #112570
[Inductor] [Quant] Re-structure Quantization testcase pattern matcher check #112570
Conversation
… check [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/112570
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (3 Unrelated Failures)As of commit ab5a900 with merge base 871e27a (): UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
… check ghstack-source-id: d504af3d68890be1a3b4d7bd36378ebf7f79858a Pull Request resolved: #112570
…ern matcher check" **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]
… check ghstack-source-id: 5b4413162ccf704e4b0fb7fd8eacb61c2bd09e65 Pull Request resolved: #112570
…ern matcher check" **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]
… check ghstack-source-id: 21cabe3c665fa1f530b0ebabc4e19d5ba2bcb884 Pull Request resolved: #112570
…ern matcher check" **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]
… check ghstack-source-id: a051f23e78aef9c3f9c1ec30b94a7f0f6101441a Pull Request resolved: #112570
Hi @jerryzh168 @jgong5, could you help to take a look of this PR again for simplifying the match checker in |
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.
LGTM. One minor suggestion FYI.
counters["inductor"]["qlinear_weight_prepack_matcher_count"], 3 | ||
) | ||
self.assertEqual( | ||
counters["inductor"]["qlinear_weight_prepack_matcher_nodes"], 18 |
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.
Do you think it's possible to have a function to calculate the number of matcher nodes from the pattern? Then, we don't have to hard-code the numbers here.
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.
Yean, I think it depends on how to write a graph analysis function with specific heuristic for calculating the expected pattern kind and pattern number given a input fx graph (after quantization flow).
If we we can get that pattern kind and numbers, then we can define a pre-known table match from pattern kind to pattern matcher node number, for example
{conv2d_weight_prepack: 6, conv2d_relu_fp32_output: 2, conv2d_relu_int8_output: 7, .....}
…ern matcher check" **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]
… check ghstack-source-id: ed686714eb513bc44da94faf7670066c3ca804fd Pull Request resolved: #112570
counters["inductor"]["qconv2d_weight_prepack_matcher_count"], 1 | ||
) | ||
self.assertEqual( | ||
counters["inductor"]["qconv2d_weight_prepack_matcher_nodes"], 6 |
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.
do you need this? what is this used for?
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.
Ideally, we only need to check matcher_count
. matcher_nodes
is a stricter restriction to check a node number in each matcher pattern.
# [convert_element_type_1, sub, mul_1, dequantize_per_channel, clone, convolution] | ||
def matcher_check_fn(): | ||
# 1. Dequant-Conv2D pattern matched in QConv2D weight prepack * 1 | ||
# [convert_element_type_1, sub, mul_1, dequantize_per_channel, clone, convolution] |
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.
how do we get this list?
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.
By printing the m.nodes
here, we can the nodes list in each matched pattern
pytorch/torch/_inductor/pattern_matcher.py
Line 1137 in 042445b
counters["inductor"]["pattern_matcher_nodes"] += len(m.nodes) |
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.
not sure why you need the matcher_nodes count
Thanks. Yean, |
@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 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 |
… check (pytorch#112570) **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` Pull Request resolved: pytorch#112570 Approved by: https://github.com/jgong5, https://github.com/jerryzh168
… check (pytorch#112570) **Summary** This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary. **TestPlan** ``` python -m pytest test_mkldnn_pattern_matcher.py -k test_q ``` Pull Request resolved: pytorch#112570 Approved by: https://github.com/jgong5, https://github.com/jerryzh168
Stack from ghstack (oldest at bottom):
Summary
This Diff re-structures Quantization testcase pattern matcher check. Instead of checking all the pattern matched in the Inductor, we will only check the core pattern match count and node numbers such as: dequant promotion, QConv/Linear Unary and QConv Binary.
TestPlan
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler