[Good First Issue][NNCF]: Remove backend-specific methods from the common layer attributes

[daniil-lyakhov]

Context

Greetings, fellow scientists😺!

Recently we've discovered an old code that becomes confusing when used with the latest algorithms:

https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/layer_attributes.py#L56-L79
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/layer_attributes.py#L135-L136

the problematic methods are get_weights_shape, get_target_dim_for_compression and get_bias_shape . Current algorithms use NNCFGraph to retrieve correct info, and not every backend follow the hardcoded weights shape layout / compression dim.

What needs to be done?

The task is to create the following functions in the https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/utils.py file:

• get_weights_shape_legacy(layer_attributes: WeightedLayerAttributes)
• get_target_dim_for_compression_legacy(layer_attributes: WeightedLayerAttributes)
• get_bias_shape_legacy(layer_attributes: WeightedLayerAttributes)

Additionally, all calls to the following methods should be replaced with their corresponding legacy function calls:

• get_weights_shape -> get_weights_shape_legacy
• get_target_dim_for_compression -> get_target_dim_for_compression_legacy
• get_bias_shape -> get_bias_shape_legacy

Furthermore, the docstrings of these methods should clarify that they are valid only for PyTorch and TensorFlow backends.

Example Pull Requests

#3179

Resources

• Contribution guide - start here!
• Intel DevHub Discord channel - engage in discussions, ask questions and talk to OpenVINO developers
• How to link your Pull Request to an issue

Contact points

@daniil-lyakhov

Ticket

No response

[shumaari]

.take

[github-actions]

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

[shumaari]

@daniil-lyakhov

I am still looking into the issue and would like to clarify my understanding:

Function calls for three methods need to be replaced by their legacy versions

layer_attributes.get_weight_shape -> layer_attributes.get_weight_shape_legacy
layer_attributes.get_target_dim_for_compression -> layer_attributes.get_target_dim_for_compression_legacy
layer_attributes.get_bias_shape -> layer_attributes.get_bias_shape_legacy

Possible locations for layer_attributes.get_weight_shape

https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/tensorflow/quantization/algorithm.py#L136
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/torch/sparsity/movement/layers.py#L170
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/torch/sparsity/movement/structured_mask_handler.py#L212
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/algo.py#L776
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/algo.py#L1185
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/init_range.py#L229
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/sparsity/const/algo.py#L29
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/sparsity/magnitude/algo.py#L47
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/sparsity/rb/algo.py#L47

Possible locations for layer_attributes.get_target_dim_for_compression

https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/common/pruning/nodes_grouping.py#L79
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/algo.py#L779
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/init_range.py#L230

Possible locations for layer_attributes.get_bias_shape

https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/torch/sparsity/movement/layers.py#L188
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/torch/sparsity/movement/structured_mask_handler.py#L214

Create the three legacy methods with docstrings

I am still unclear about where these functions could be created? Within class definitions for WeightedLayerAttributes and LinearLayerAttributes classes in https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/layer_attributes.py like below or somewhere else?

I would also like to mention that get_weight_shape_legacy and get_target_dim_for_compression_legacy methods in WeightedLayerAttributes class are mere placeholders. The method which caused the reported error in #3230 (comment) must be defined elsewhere.

I intend to recreate the error reported above. What else can I do to investigate further?

[daniil-lyakhov]

Hi @shumaari,

List of the method usage is correct, thanks for your analysis. I'm apologies for not clear instructions, the task is to create 3 functions (not class methods) in the https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/utils.py file. Then, the assignee have to replate method calls with the functions calls.

Please don't spend too much time on the error reproduction: it isn't a error indeed, it is a wrong usage of a backend-specific method

[shumaari]

Hi @daniil-lyakhov

Thank you for the clarifying the location of new (legacy) methods.

I am still looking into the issue and would like to confirm that I am on the right track:

1. (as shared before) There are 14 possible locations for corrections, 9 for layer_attributes.get_weight_shape, 3 for layer_attributes.get_target_dim_for_compression, 2 for layer_attributes.get_bias_shape
2. The locations are split as follows:
   a. 4 in https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/torch/ folder (torch backend)
   b. 8 in https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/ folder (torch backend)
   c. 1 in https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/tensorflow/ folder (tensorflow backend)
   d. 1 in https://github.com/openvinotoolkit/nncf/blob/develop/nncf/experimental/common/pruning/ folder (common to both torch and tensorflow backends)

Question: The methods need to be replaced in all these four contexts?

3. In each of these 14 locations, the method argument layer_attributes is the value for "layer_attributes" key in the _attributes dictionary for NNCFNode object
4. This layer_attributes value is an instance of BaseLayerAttributes class or one of its subclasses. However, the methods may differ based on the subclass.

Question: Should the new (legacy) methods contain branched logic based on which subclass the instance belongs to, or should it contain code that is applicable directly to all subclasses? (I am not sure if the latter is actually possible.) If branched logic needs to be followed, can the relevant class methods be called or does the code need to be self-contained?

5. In some of the locations it is not immediately apparent which subclass of BaseLayerAttributes layer_attributes belongs to.

Question: Is it safe to assume that it belongs to one of the two subclasses, GenericWeightedLayerAttributes or LinearLayerAttributes unless explicitly clear otherwise?

Thank you for your patience and guidance!

[daniil-lyakhov]

Hi @shumaari, let me answer your questions in form of a pseudocode:
in file https://github.com/openvinotoolkit/nncf/blob/develop/nncf/common/graph/utils.py

def get_weights_shape_legacy(layer_attributes: GenericWeightedLayerAttributes):
    if isinstance(layer_attributes, LinearLayerAttribute):
        # Code copied from the LinerLayerAttribute
    if isinstance(layer_attributes, ConvolutionLayerAttribute):
        # Code copied from the ConvolutionLayerAttribute
    # etc for each child of the GenericWeightedLayerAttributes
    ....

def get_target_dim_for_compression_legacy(layer_attributes: GenericWeightedLayerAttributes):
    # Same logic as above
    ...

def get_bias_shape_legacy(layer_attributes: GenericWeightedLayerAttributes):
    # Same logic as above
    ....

Then the removed layer attributes methods should be replaced like this:

layer_attribute.get_weights_shape() -> get_weights_shape_legacy(layer_attribute)

Please don't list all the places you are going to update, just open a draft PR with your changes. It would be easier to comment on the actual code.

Regarding 3 and 4: there are different layer attribute classes in the NNCF, please replace the usage of the methods only for those which are parents of GenericWeightedLayerAttributes and keep all other layer attributes as it is. BaseLayerAttributes does not have get_weights_shape, get_target_dim_for_compression and get_bias_shape methods, thus you shouldn't change it

[shumaari]

@daniil-lyakhov

Apologies for the clutter above. I have created a draft pull request 3287 . Could you please provide feedback?

Thank you!

[daniil-lyakhov]

@shumaari, thank you for your contribution! We are looking forward to collaborate with you in future!