[quant][graphmode] Move interpolate to general value ops#38162
[quant][graphmode] Move interpolate to general value ops#38162jerryzh168 wants to merge 8 commits intogh/jerryzh168/305/basefrom
Conversation
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit 31b92e9 (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).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker. This comment has been revised 18 times. |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
| general_value_op_quant = 14 | ||
| FileCheck().check_count("aten::quantize_per_tensor(", conv_op_quant + general_value_op_quant + 1, exactly=True) \ | ||
| # number of quantize_per_tensor op for type | ||
| num_quant_by_op_type = {'conv': 2, 'common': 1, 'interpolate': 3} |
There was a problem hiding this comment.
Can you explain why there are three quantize_per_tensor ops per interpolate op?. Is this because it calls different types of upsampling ops and each of them needs a separate quantize_per_tensor op at the output?
There was a problem hiding this comment.
there are some if statement in the code, and we have one quantize_per_tensor for each callsite of the aten ops inside if.
| # number of quantize_per_tensor op for type | ||
| num_quant_by_op_type = {'conv': 2, 'common': 1, 'interpolate': 3} | ||
| # number of ops for each type | ||
| num_op_by_op_type = {'conv': 2, 'common': 16, 'interpolate': 3} |
There was a problem hiding this comment.
It looks like there are 5 interpolate ops?
There was a problem hiding this comment.
probably need to rename this, 'interpolate' just means the ops that have three quantize_per_tensor..
There was a problem hiding this comment.
will rename this in later PR
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
| # number of quantize_per_tensor op for type | ||
| num_quant_by_op_type = {'conv': 2, 'common': 1, 'interpolate': 3} | ||
| # number of ops for each type | ||
| num_op_by_op_type = {'conv': 2, 'common': 16, 'interpolate': 3} |
There was a problem hiding this comment.
What does common mean here?
There was a problem hiding this comment.
this is refactored in later PRs, 'common' just mean typical ops..
There was a problem hiding this comment.
typical op will introduce 1 quantize_per_tensor call in the IR.
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
|
stamping per @jerryzh168 's request |
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
|
This pull request has been merged in e988b4f. |
Summary: Pull Request resolved: pytorch#38162 Test Plan: Imported from OSS Differential Revision: D21559810 fbshipit-source-id: 2d975fc71f73c18f594108172850dfcfdb0cb9a0
Stack from ghstack:
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D21559810