[ONNX][SmoothQuant] Introduce a new keep_axes parameter #3687
Conversation
|
|
||
| @staticmethod | ||
| def get_abs_max_reducer_cls() -> type[OVAbsMaxReducer]: | ||
| return OVAbsMaxReducer | ||
|
|
||
| @staticmethod | ||
| def get_shape_reducer_cls() -> type[OVShapeReducer]: | ||
| return OVShapeReducer |
There was a problem hiding this comment.
We add the get_abs_max_reducer_cls() and get_shape_reducer_cls() methods here because the OpenVINO backend uses the OVAbsMaxReducer and OVShapeReducer classes instead of AbsMaxReducer and ShapeReducer to enable in-place statistic collection.
andrey-churkin
changed the title
andrey-churkin
added
NNCF Common
Thank you for the suggestion. I’ll consider how to implement it. UPD: This problem is reproduced on timm/visformer_small model from the ptq scope. |
| def __init__(self, reduction_axes: Optional[ReductionAxes] = None, inplace: bool = False): | ||
| def __init__( | ||
| self, | ||
| reduction_axes: Optional[ReductionAxes] = None, |
There was a problem hiding this comment.
Should we forward this parameter in the children of the TensorReducerBase?
| def __init__( | ||
| self, | ||
| reduction_axes: Optional[ReductionAxes] = None, | ||
| keep_axes: Optional[tuple[int, ...]] = None, |
There was a problem hiding this comment.
| keep_axes: Optional[tuple[int, ...]] = None, | |
| keep_axes: Optional[Axes] = None, |
Perhaps we could rename ReductionAxes and reuse them there?
|
|
||
| def __hash__(self) -> int: | ||
| return hash((self.__class__.__name__, self.inplace, self._reduction_axes)) | ||
| return hash((self.__class__.__name__, self.inplace, self._reduction_axes, self._keep_axes)) |
There was a problem hiding this comment.
Perhaps we should update __hash__ methods for some of the TensorReducerBase as well
|
|
||
| def test_get_abs_max_channel_collector(self, inplace_statistics: bool): | ||
| backend = self.get_backend() | ||
| reduction_axes = (3, 2, 1) |
There was a problem hiding this comment.
Please test self._backend_entity.get_abs_max_reducer_cls() and _backend_entity.get_shape_reducer_cls
| if model_backend == BackendType.ONNX: | ||
| keep_axes = (self._backend_entity.get_activation_channel_axis(node_to_smooth, input_act_port),) | ||
| reduction_axes = None | ||
| else: | ||
| keep_axes = None | ||
| reduction_axes = self._calculate_input_reduction_axes(graph, node_to_smooth, input_act_port) |
There was a problem hiding this comment.
Usually we create a method in the backend to resolve such situation, why don't you introduce a method in the backend? The comment could be placed as a docstring for the method
| ): | ||
| stats = tensor_collector.get_statistics() | ||
| shape = stats[SHAPE_BRANCH_KEY] | ||
| shape = tuple() if shape is None else tuple(shape.tolist()) |
There was a problem hiding this comment.
When shape could be None?
| self._reduction_axes = reduction_axes | ||
| self._keep_axes = keep_axes |
There was a problem hiding this comment.
Do these 2 variables mutually exclusive?
If yes, I'd rename _reduction_axes to _axes and added _axes_mode - REDUCTION or KEEP
4 participants
Changes
This PR introduces a new
keep_axesparameter forTensorReducerBase. This parameter specifies the axes to preserve during the reduction operation. It is used to calculate the reduction axes during statistic collection, allowing us to avoid requiring the actual tensor shape (actually only number of dimensionsndimis required) before inference.Modifies the
SmoothQuantalgorithm to use thekeep_axesparameter for the ONNX backend instead of relying on the tensor shape from the NNCF graph, as this shape isn't always available.Related tickets
Ref: 173880, Ref: 174334
Tests