[PTQ][OV] Fix inplace statistic reducers names and hashes #1796
Conversation
github-actions
bot
added
experimental
NNCF OpenVINO
daniil-lyakhov
force-pushed
the
dl/ov/fix_names_in_inplace_stats
branch
from
e1b3883
to
52f5100
Compare
github-actions
bot
added
NNCF Common
daniil-lyakhov
force-pushed
the
dl/ov/fix_names_in_inplace_stats
branch
3 times, most recently
from
7981448
to
15c38fa
Compare
|
Conformance tests showed no regressions |
| @@ -39,6 +39,7 @@ def __init__(self, reduction_shape: Optional[ReductionShape] = None, inplace: bo | |||
|
|
|||
| """ | |||
| self._reduction_shape = reduction_shape | |||
| self._init_reduction_shape = reduction_shape | |||
There was a problem hiding this comment.
What is the difference between self._reduction_shape and self._init_reduction_shape? They have the same value, as I can see.
There was a problem hiding this comment.
The problem is that reduction_shape is needed to calculate reducer hash value to distinguish two different reducer with equal class. In case reduction shape is None at the beginning, it's calculated on first iteration
nncf/nncf/experimental/common/tensor_statistics/collectors.py
Lines 94 to 95 in 16d18e8
TensorCollector will be invalid after the first iteration. This _init_reduction_shape doesn't change during reducer life so it's safe for hash calculation
There was a problem hiding this comment.
It looks like if reduction_shape is None than the reducer should reduce by all dimensions. I suggest to implement this logic in each reducer not in the base class. You can implement the method to create reduction shape by tensor and use it in any inhered class.
There was a problem hiding this comment.
Done, please check
daniil-lyakhov
force-pushed
the
dl/ov/fix_names_in_inplace_stats
branch
from
16d18e8
to
e9f9c8d
Compare
| @@ -39,6 +39,7 @@ def __init__(self, reduction_shape: Optional[ReductionShape] = None, inplace: bo | |||
|
|
|||
| """ | |||
| self._reduction_shape = reduction_shape | |||
| self._init_reduction_shape = reduction_shape | |||
There was a problem hiding this comment.
It looks like if reduction_shape is None than the reducer should reduce by all dimensions. I suggest to implement this logic in each reducer not in the base class. You can implement the method to create reduction shape by tensor and use it in any inhered class.
|
Full check showed on regression on this PR, it safe to merge it to develop and release |
…1827) Copy of #1796 ### Changes Names of reducers is used instead of types of reducers to name statistic subnet operations Reducer names now differs one from another if they have different attributes and aren't merged Hash of reducer now take into account reduction_shape ### Reason for changes To fix problem when two different statistics that are instances of one class and share target point have equal name ### Related tickets 109989 ### Tests Test on reducers merging Test with a model that have target point with two different reducers that are instances of one class --------- Co-authored-by: Nikita Savelyev <nikita.savelyev@intel.com>
Changes
reduction_shapeReason for changes
To fix problem when two different statistics that are instances of one class and share target point have equal name
Related tickets
109989
Tests
TODO: