Fix output spec adjustment for fixed qparams ops - #22
Conversation
3fac694 to
462226c
Compare
462226c to
8550f72
Compare
u-simha
left a comment
u-simha
left a comment
There was a problem hiding this comment.
Looks good, few minor comments
| torch.ops.aten.sigmoid_.default: _sigmoid_qspec, | ||
| torch.ops.aten.hardsigmoid.default: _sigmoid_qspec, | ||
| torch.ops.aten.hardsigmoid_.default: _sigmoid_qspec, | ||
| torch.ops.aten.tanh.default: (QuantizationScheme.SYMMETRIC, (-1.0, 1.0)), |
There was a problem hiding this comment.
Might be useful to have single line comments here (also acting as separators) - similar to what we have on line 162 & 165 for relu & relu6. This would help improve readability.
| assert fq_partial_out.dtype == x.dtype | ||
| assert fq_direct_out.dtype == x.dtype | ||
|
|
||
| def test_update_partial_qparams_calculator_single_attr(self): |
There was a problem hiding this comment.
I think couple of these tests can be combined into a single one?
| return spec.fake_quantize_cls(**common_args, **extra_args) | ||
|
|
||
| @classmethod | ||
| def update_partial_qparams_calculator( |
There was a problem hiding this comment.
If we are returning a new partial, I wonder if it is better to call it something other than "update"? probably "reconstruct" - just so that it is obvious that the existing one is not updated (partials anyway can not be updated I'm guessing)
8550f72 to
e9e8e7b
Compare
There was a problem hiding this comment.
We should use 22.fixed instead of 180525445.fixed as the file name, where "22" is the PR number from GitHub.
|
4 participants
This PR fixes the output qspec adjustment behavior for ops with known ranges like relu, relu6, sigmoid, tanh, etc.
Previously the code did not properly perform the adjustment. Qscheme was not being set correctly, and no fixed ranges were ever in place.
This PR handles correctly setting the qscheme and float range as applicable for each op.
Refactoring was also done to remove the independent qscheme attribute of fake quantize, instead having it read directly from its qparams_calculator.qscheme instead.
Note: the mnist test accuracy expectation post-prepare was updated from < 88% to < 94%. I checked that this large difference was due only to the hardtanh output quantizers now being set to asymmetric with float range (0.0, 1.0).