[pt][quant] Update the misleading comments for zero_points and scale in dynamic quant linear module #28767
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
…in dynamic quant linear module The scale and zero_point are for the output activation tensor, not for the weight tensor; Differential Revision: [D18164949](https://our.internmc.facebook.com/intern/diff/D18164949/) [ghstack-poisoned]
jianyuh
added a commit
that referenced
this pull request
Oct 27, 2019
…in dynamic quant linear module The scale and zero_point are for the output activation tensor, not for the weight tensor; Differential Revision: [D18164949](https://our.internmc.facebook.com/intern/diff/D18164949/) ghstack-source-id: 92712041 Pull Request resolved: #28767
z-a-f
approved these changes
Oct 28, 2019
| scale: `scale` parameter of weight Quantized Tensor, type: double | ||
| zero_point: `zero_point` parameter for weight Quantized Tensor, type: long | ||
| scale: `scale` parameter of output activation Quantized Tensor, type: double | ||
| zero_point: `zero_point` parameter for output activation Quantized Tensor, type: long |
There was a problem hiding this comment.
There is no reason to capitalize the "Quantized Tensor" -- let's keep it as "quantized tensor"
raghuramank100
suggested changes
Oct 28, 2019
Contributor
raghuramank100
left a comment
raghuramank100
left a comment
There was a problem hiding this comment.
I think this comment is misleading, please see my comments below
| If :attr:`bias` is ``True``, the values are initialized to zero. | ||
| scale: `scale` parameter of weight Quantized Tensor, type: double | ||
| zero_point: `zero_point` parameter for weight Quantized Tensor, type: long | ||
| scale: `scale` parameter of output activation Quantized Tensor, type: double |
Contributor
There was a problem hiding this comment.
For dynamic quantization there is no output scale and zero-point. We should not be exposing these in the comments. The activations are in floating point.
Member
Author
There was a problem hiding this comment.
We have this PR, mainly because when we print the modules, we print the scale and zero points:
https://github.com/pytorch/pytorch/blob/master/torch/nn/quantized/modules/linear.py#L47-L48
One example is for the RoBERTa model after dynamic quantization:
(19): TransformerEncoderLayer(
(dropout): Dropout(p=0.1, inplace=False)
(attention): MultiheadAttention(
(dropout): Dropout(p=0.1, inplace=False)
(input_projection): DynamicQuantizedLinear(in_features=1024, out_features=3072, scale=1.0, zero_point=0)
(output_projection): DynamicQuantizedLinear(in_features=1024, out_features=1024, scale=1.0, zero_point=0)
)
(residual_mlp): ResidualMLP(
(mlp): Sequential(
(0): DynamicQuantizedLinear(in_features=1024, out_features=4096, scale=1.0, zero_point=0)
(1): GeLU()
(2): Dropout(p=0.1, inplace=False)
(3): DynamicQuantizedLinear(in_features=4096, out_features=1024, scale=1.0, zero_point=0)
(4): Dropout(p=0.1, inplace=False)
)
)
(attention_layer_norm): LayerNorm((1024,), eps=1e-05, elementwise_affine=True)
(final_layer_norm): LayerNorm((1024,), eps=1e-05, elementwise_affine=True)
)
(20): TransformerEncoderLayer(
(dropout): Dropout(p=0.1, inplace=False)
(attention): MultiheadAttention(
(dropout): Dropout(p=0.1, inplace=False)
(input_projection): DynamicQuantizedLinear(in_features=1024, out_features=3072, scale=1.0, zero_point=0)
(output_projection): DynamicQuantizedLinear(in_features=1024, out_features=1024, scale=1.0, zero_point=0)
)
(residual_mlp): ResidualMLP(
(mlp): Sequential(
(0): DynamicQuantizedLinear(in_features=1024, out_features=4096, scale=1.0, zero_point=0)
(1): GeLU()
(2): Dropout(p=0.1, inplace=False)
(3): DynamicQuantizedLinear(in_features=4096, out_features=1024, scale=1.0, zero_point=0)
(4): Dropout(p=0.1, inplace=False)
)
)
(attention_layer_norm): LayerNorm((1024,), eps=1e-05, elementwise_affine=True)
(final_layer_norm): LayerNorm((1024,), eps=1e-05, elementwise_affine=True)
)
In the comment, we would like to tell the users that the scale=1.0 and zero_point=0 for DynamicQuantizedLinear module above are for the output activation (to be consistent with the static quantization).
… and scale in dynamic quant linear module" The scale and zero_point are for the output activation tensor, not for the weight tensor; Differential Revision: [D18164949](https://our.internmc.facebook.com/intern/diff/D18164949/) [ghstack-poisoned]
Member
Author
|
Per @raghuramank100 's request, we removed the comments for |
raghuramank100
approved these changes
Oct 29, 2019
Contributor
|
This pull request has been merged in b1ea19c. |
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:
The scale and zero_point are for the output activation tensor, not for the weight tensor;
Differential Revision: D18164949