-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[quantization] Store bias in PackedConvWeight in fbgemm #25626
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
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/) ghstack-source-id: 89450557 Pull Request resolved: #25626
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89452640 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89454242 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
| for (int i = 0; i < K; ++i) { | ||
| bias_scale.data_ptr<double>()[i] = pack_ptr.w_scale[i] * act_scale; | ||
| } | ||
| qbias = quantize_linear_per_channel_cpu( |
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.
quantize_linear_per_channel instead of quantize_linear_per_channel_cpu
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.
Using the quantize_linear_per_channel function causes the following error
RuntimeError: Expected object of type Variable but found type CPUFloatType for argument #1 'scales' (checked_cast_variable at caffe2/torch/csrc/autograd/VariableTypeManual.cpp:38)
Spoke to @yf225 offline and he said the error suggests that we are missing a at::AutoNonVariableTypeMode thread-local guard somewhere in the call path of QLinearInt8 operator().
quantize_linear_per_channel dispatches to VariableType dispatcher, which expects the input tensor to be a Variable.
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 know why are we missing this guard?
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.
I am not sure. @smessmer any thoughts on if this guard should be added?
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.
Is this issue resolved?
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 yet, I will file a separate issue for this.
| stride=stride, padding=padding, dilation=dilation, | ||
| groups=groups, bias=bias, padding_mode=padding_mode) | ||
|
|
||
| def weight(self): |
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.
Are these two methods being removed?
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.
Yes, I don't see why they are needed as the parent module already defined them. Looks like linear_relu.py also doesn't have them
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89519225 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89559766 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89601358 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89630144 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
raghuramank100
left a comment
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.
Looks great, please address comments prior to submitting.
Add bias as an optional parameter in the packed conv weight struct. Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
Pull Request resolved: #25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89780639 Differential Revision: [D17177723](https://our.internmc.facebook.com/intern/diff/D17177723/)
|
This pull request has been merged in c60dddb. |
Summary: Pull Request resolved: pytorch/pytorch#25626 Add bias as an optional parameter in the packed conv weight struct. ghstack-source-id: 89780639 Test Plan: python test/run_test.py --exclude nn --verbose --bring-to-front quantization quantized quantized_tensor quantized_nn_mods quantizer Reviewed By: raghuramank100 Differential Revision: D17177723 fbshipit-source-id: e502f2196cb1c002db8b691124db740368944c92
Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: [ghstack-poisoned]
…rns" Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: [ghstack-poisoned]
…rns" Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: [ghstack-poisoned]
…rns" Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: [ghstack-poisoned]
…rns" Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: [ghstack-poisoned]
…rns" Summary: Fix the patterns after changes to prepack functions(#25626) Test Plan: pytho test/test_jit.py 'TestJit.test_quant_fusion' Reviewers: mvz Subscribers: Tasks: Tags: Differential Revision: [D17465553](https://our.internmc.facebook.com/intern/diff/D17465553) [ghstack-poisoned]
Stack from ghstack:
Add bias as an optional parameter in the packed conv weight struct.
Differential Revision: D17177723