Invalidate irrelevant configuration in node quantization configs#1495
Invalidate irrelevant configuration in node quantization configs#1495
Conversation
… quantization configs attributes if quantization is disabled, NodeQuantizationConfig removes duplicates automatically.
irenaby
force-pushed
the
unset_disabled_quant
branch
from
c58c921 to
e38e426
Compare
| assert cfg.weights_channels_axis is None | ||
|
|
||
|
|
||
| class TestWeightsQuantizationConfig: |
There was a problem hiding this comment.
No change from here to line 224 (file was moved)
irenaby
force-pushed
the
unset_disabled_quant
branch
from
e38e426 to
9c76b53
Compare
irenaby
changed the title
|
|
||
| validate: InitVar[bool] = True | ||
|
|
||
| def __post_init__(self, validate=True): |
There was a problem hiding this comment.
Can __post_init__ take input arguments?
| def _unset(self): | ||
| """ Unset activation quantization fields to None. """ | ||
| self.activation_quantization_method = None | ||
| self.activation_n_bits = 0 |
There was a problem hiding this comment.
There several places where we look for max/min candidate or sort based on nbits whether quantization is enabled or not, which still work with 0 but not with None. I wanted to minimize changes elsewhere and 0 is clearly not a real value. That is the reason, but it's probably not a good enough reason.
| return attr_cfg | ||
|
|
||
| def set_attr_config(self, attr_name: 'WeightAttrT', attr_qc: WeightsAttrQuantizationConfig): | ||
| def set_attr_config(self, attr_name: 'WeightAttrT', attr_qc: WeightsAttrQuantizationConfig, force=False): |
| f"Weights attribute {attr_name} could not be found to set parameter {config_parameter_name}.") | ||
|
|
||
| attr_cfg = self.get_attr_config(attr_name) | ||
| if config_parameter_name == 'enable_weights_quantization': |
There was a problem hiding this comment.
Can replace str with constant?
There was a problem hiding this comment.
It's not a const, it's a name of an attribute in the same class, it's meaningless to define it as const
| AttributeQuantizationConfig( | ||
| enable_weights_quantization=False))) | ||
| enable_weights_quantization=False)), | ||
| force=True) |
There was a problem hiding this comment.
Why is force needed? Add comment for it
There was a problem hiding this comment.
Because the attribute doesn't exist in the config. Without force it will raise an error. Will add a comment.
| WeightsAttrQuantizationConfig(AttributeQuantizationConfig( | ||
| enable_weights_quantization=False))) | ||
| enable_weights_quantization=False)), | ||
| force=True) |
2 participants
Pull Request Description:
Un-set configuration in node quantization configs if it is or becomes irrelevant. This was we don't hold invalid params, prevent using them by mistake, and removing redundant candidates becomes much easier (given that we don't want to completely restructure the configs at this point). NodeQuantizationConfig removes duplicate candidates automatically.
Replace old tests for nodes configs.
Checklist before requesting a review: