Refactor NNCF compression statistics reporting API #688

andrey-churkin · 2021-04-30T01:46:17Z

No description provided.

vshampor · 2021-04-30T04:51:12Z

nncf/api/compression.py

+        """
+        Returns a representation of the statistics as built-in data types.
+
+        :return: A representation of the statistics as built-in data types.


Should add a note on what the string keys in the dict represent.

The as_dict() method was removed after refactoring. For this reason, your comment is not applicable now.

vshampor · 2021-04-30T04:53:15Z

nncf/common/pruning/statistics.py

+            'mask_pruning_level': self.mask_pruning_level,
+            'filter_pruning_level': self.filter_pruning_level,
+        }
+        return summary


Note that this is essentially a copy-and-paste of the corresponding self.__dict__. If you decide to take this approach, then perhaps it would be easier to write a common Statistics function that iterates over self.__dict__, puts built-in types (int, double and str) into the retval dict as-is, puts the result of .as_dict for Statistics entries and ignores the rest (if encountered, spawn a debug message with a warning).

The as_dict() method was removed after refactoring. I implemented a helper function that prepares statistics for the tensor board.

vshampor · 2021-04-30T04:58:13Z

examples/classification/main.py

@@ -453,7 +453,7 @@ def train_epoch(train_loader, model, criterion, criterion_fn, optimizer, compres
            config.tb.add_scalar("train/top1", top1.avg, i + global_step)
            config.tb.add_scalar("train/top5", top5.avg, i + global_step)

-            for stat_name, stat_value in compression_ctrl.statistics(quickly_collected_only=True).items():
+            for stat_name, stat_value in compression_ctrl.statistics(quickly_collected_only=True).as_dict().items():


Can the tb.add_scalar handle nested dicts? Because your as_dict structure is, in general, nested.

The tf.add_scalar is not support nested dicts.

alexsu52 · 2021-05-13T09:00:08Z

beta/examples/tensorflow/classification/main.py

@@ -221,7 +220,7 @@ def run(config):
            **validation_kwargs)

    logger.info('evaluation...')
-    print_statistics(compression_ctrl.statistics())
+    logger.info(compression_ctrl.statistics().as_str())


Suggested change

logger.info(compression_ctrl.statistics().as_str())

compression_statistics = compression_ctrl.statistics()

logger.info(compression_statistics.as_str())

If you support this change. Please update the same code snippets below too.

alexsu52 · 2021-05-13T09:25:15Z

beta/nncf/tensorflow/pruning/base_algorithm.py

-            })
-
-        return raw_pruning_statistics
+            # TODO(andrey-churkin): Should be calculated


Could you file a ticket for this TODO?

I created a ticket (55672) to track the progress of this task.

alexsu52 · 2021-05-13T09:46:11Z

beta/nncf/tensorflow/sparsity/rb/algorithm.py

        sparsity_levels = tf.keras.backend.batch_get_value(sparsity_levels)
        weights_percentages = [weights_number / total_weights_number * 100
                               for weights_number in weights_numbers]
        weights_percentages = tf.keras.backend.batch_get_value(weights_percentages)
        mask_sparsity = list(zip(mask_names, weights_shapes, sparsity_levels, weights_percentages))
-        raw_sparsity_statistics['sparsity_statistic_by_layer'] = []
+
+        # TODO(andrey-churkin): Why we use `mask_name` instead of `layer_name`?


@daniil-lyakhov Could you comment?

It's name from magnitude sparsity raw_statistic method. I used wrapped_layer.name + '_rb_mask' name because it contains name of the layer and mask.name consist of weight_attribute + '_mask' thus less user friendly. mask_name not actually contains mask.names but contains their user frendly names

Seems like a bug because NNCFWrapper can contain multiple operations with weights.

alexsu52 · 2021-05-13T09:54:14Z

beta/nncf/tensorflow/sparsity/rb/algorithm.py

+
+        target_level = self.loss.target_sparsity_rate
+        # TODO(andrey-churkin): Should be calculated when the distributed mode will be supported
+        masks_consistency = 1.0


We covered this case in tests. So this statistics does not mean sense for TF backend.

@vshampor Do we really want to report the masks_consistency statistic to users?

@vshampor Could you please answer the question above?

I have had no idea what mask consistency is before you had drawn my attention to it, and it is not immediately obvious to me what this parameter means in the context of sparsity and how it impacts the sparsity outputs. I think that we can omit reporting it for now.

alexsu52 · 2021-05-13T09:54:55Z

beta/nncf/tensorflow/sparsity/rb/algorithm.py

+        # TODO(andrey-churkin): Should be calculated when the distributed mode will be supported
+        masks_consistency = 1.0
+
+        # TODO(andrey-churkin): Check that `mean_sparse_prob` is calculated correctly


Could you check it in this PR or file a ticket?

Fixed in ac19dc6.

alexsu52 · 2021-05-13T13:35:05Z

nncf/api/composite_compression.py

@@ -23,6 +24,38 @@
 ModelType = TypeVar('ModelType')


+class CompositeStatistics(Statistics):


If we look in terms of the main usage scenarios. I guess, It is access to statistics values. We will see that the CompressionAlgorithmController returns Statistics object, but the CompositeCompressionAlgorithmController returns CompositeStatistics object. Currently, If the developer want to get sparsity_level, for example, then he should write the following code:

statistics = compression_ctrl.statistics() if isinstance(statistics, CompositeStatistics): items = statistics.child_statistics else: items = [statistics] for item in items: if isinstance(item, (MagnitudeSparsityStatistics, RBSparsityStatistics, ConstSparsityStatistics)): sparsity_level = item.model_statistics.sparsity_level

before these changes it was like this:

statistics = compression_ctrl.statistics() sparsity_level = statistics['sparsity_level']

I was expecting the following:

statistics = compression_ctrl.statistics() sparsity_level = statistics.magnitude_sparsity.sparsity_level #or magnitude_sparsity_statistics = statistics.get_statistics('magnitude_sparsity') sparsity_level = magnitude_sparsity_statistics.sparsity_level

Can we make the statistics method return one type of object? Statistics container? @andrey-churkin, @vshampor Could you look once more at the proposal in this PR in terms of UX/DX?

Thank you very much for your suggestion. I fixed it in b5398b0.

andrey-churkin · 2021-05-20T09:02:04Z

Important changes:

The nncf/api/composite_compression.py file was moved to nncf/common/composite_compression.py.
The statistics() method was removed from CompressionLoss class.
The NNCFStatistics class was introduced. All controllers should return an instance of this class from the statistics() method.

vshampor · 2021-05-20T17:10:02Z

Jenkins please retry a build

asenina

👍

andrey-churkin · 2021-05-23T20:57:27Z

Jenkins please retry a build

andrey-churkin · 2021-05-23T23:30:09Z

@vshampor Could you please look at this PR?

* Refactor NNCF compression statistics reporting API * Test was updated * Fix pylint * Refactoring * Minor fixes * Minor fixes * Minor updates * Minor updates * Fixed mean_sparse_prob calculation * Refactoring * Typo was fixed * Test was updated * Test was updated * Fix pylint

andrey-churkin added the NNCF Common Pull request that updates NNCF Common label Apr 30, 2021

andrey-churkin requested review from alexsu52, vshampor and a team April 30, 2021 01:46

andrey-churkin marked this pull request as draft April 30, 2021 01:47

vshampor reviewed Apr 30, 2021

View reviewed changes

andrey-churkin force-pushed the ac/stats branch from c3368ac to 8e2de8a Compare May 12, 2021 13:47

andrey-churkin marked this pull request as ready for review May 13, 2021 08:05

andrey-churkin requested a review from asenina May 13, 2021 08:26

alexsu52 requested changes May 13, 2021

View reviewed changes

andrey-churkin requested review from alexsu52, vshampor and daniil-lyakhov May 20, 2021 00:23

andrey-churkin force-pushed the ac/stats branch from b5398b0 to 4840849 Compare May 20, 2021 08:35

alexsu52 approved these changes May 20, 2021

View reviewed changes

andrey-churkin force-pushed the ac/stats branch from 4840849 to 6c3ff32 Compare May 20, 2021 22:32

asenina approved these changes May 21, 2021

View reviewed changes

andrey-churkin force-pushed the ac/stats branch from e154da5 to afc0572 Compare May 21, 2021 10:54

andrey-churkin added 10 commits May 23, 2021 22:32

Refactor NNCF compression statistics reporting API

62d5fe9

Test was updated

37c0f87

Fix pylint

3498268

Refactoring

38708d3

Minor fixes

b2f1210

Minor fixes

dc53fb4

Minor updates

6395ba8

Minor updates

39edd9e

Fixed mean_sparse_prob calculation

054431c

Refactoring

0095477

andrey-churkin added 4 commits May 23, 2021 22:41

Typo was fixed

06ff0b6

Test was updated

a81f685

Test was updated

4f5b65d

Fix pylint

d0cde1f

andrey-churkin force-pushed the ac/stats branch from dd394a6 to d0cde1f Compare May 23, 2021 19:42

vshampor merged commit 018d5dc into openvinotoolkit:develop May 24, 2021

evgeniya-egupova mentioned this pull request May 24, 2021

Fix raw statistics for filter pruning #717

Closed

l-bat mentioned this pull request May 25, 2021

Fixed third_party patches #732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor NNCF compression statistics reporting API #688

Refactor NNCF compression statistics reporting API #688

andrey-churkin commented Apr 30, 2021

vshampor Apr 30, 2021

andrey-churkin May 14, 2021

vshampor Apr 30, 2021

andrey-churkin May 14, 2021

vshampor Apr 30, 2021

andrey-churkin May 14, 2021

alexsu52 May 13, 2021

andrey-churkin May 14, 2021

alexsu52 May 13, 2021

andrey-churkin May 16, 2021

alexsu52 May 13, 2021

daniil-lyakhov May 14, 2021 •

edited

Loading

andrey-churkin May 15, 2021

alexsu52 May 13, 2021

andrey-churkin May 14, 2021

andrey-churkin Jun 10, 2021

vshampor Jun 10, 2021

alexsu52 May 13, 2021

andrey-churkin May 15, 2021

alexsu52 May 13, 2021

andrey-churkin May 20, 2021

andrey-churkin commented May 20, 2021

vshampor commented May 20, 2021

asenina left a comment

andrey-churkin commented May 23, 2021

andrey-churkin commented May 23, 2021

	logger.info(compression_ctrl.statistics().as_str())
	compression_statistics = compression_ctrl.statistics()
	logger.info(compression_statistics.as_str())

		@@ -23,6 +24,38 @@
		ModelType = TypeVar('ModelType')


		class CompositeStatistics(Statistics):

Refactor NNCF compression statistics reporting API #688

Refactor NNCF compression statistics reporting API #688

Conversation

andrey-churkin commented Apr 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniil-lyakhov May 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrey-churkin commented May 20, 2021

vshampor commented May 20, 2021

asenina left a comment

Choose a reason for hiding this comment

andrey-churkin commented May 23, 2021

andrey-churkin commented May 23, 2021

daniil-lyakhov May 14, 2021 •

edited

Loading