Skip to content

Tensor stats migration #3440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

darshil929
Copy link
Contributor

Changes

Reason for changes

Related tickets

Tests

@github-actions github-actions bot added NNCF TF Pull requests that updates NNCF TensorFlow NNCF PT Pull requests that updates NNCF PyTorch experimental NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ labels Apr 17, 2025
@darshil929
Copy link
Contributor Author

Hi @kshpv @alexsu52

This PR is related to the issue #3041 and the changes implemented are:

  • Removed circular dependencies between old and new implementations
  • Migrated statistics classes imports to use experimental versions
  • Updated tensor collector references to use the experimental implementation
  • Updated PyTorch and OpenVINO implementations
  • Added certain comments for my reference which will be removed during the final cleanup if necessary

Current status:

  • Old implementation is now importing experimental TensorStatistic
  • Framework-specific implementations are partially updated

I noticed the experimental implementation is missing:

  • statistic_point.py (StatisticPoint and StatisticPointsContainer)
  • aggregator.py (StatisticsAggregator)
  • statistics_serializer.py and statistics_validator.py

These components are widely used throughout the codebase (30+ files).

Could you please advice if I should implement these missing components as part of the experimental implementation, or is there a different approach you would recommend?

@darshil929
Copy link
Contributor Author

Hi @alexsu52

Sir, could you please have a look at the previous comment and advice me on how I should proceed?

@@ -28,6 +27,8 @@
from nncf.tensor import Tensor

InplaceInsertionFNType = TypeVar("InplaceInsertionFNType")
# AggregationAxes serves the same purpose as ReductionAxes in the old implementation
# but is defined here to break circular dependencies
AggregationAxes = Tuple[int, ...]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this help with cyclical dependencies?

Suggested change
AggregationAxes = Tuple[int, ...]
AggregationAxes = Tuple[int, ...]
ReductionAxes = Tuple[int, ...]

@alexsu52
Copy link
Contributor

Hi @kshpv @alexsu52

This PR is related to the issue #3041 and the changes implemented are:

* Removed circular dependencies between old and new implementations

* Migrated statistics classes imports to use experimental versions

* Updated tensor collector references to use the experimental implementation

* Updated PyTorch and OpenVINO implementations

* Added certain comments for my reference which will be removed during the final cleanup if necessary

Current status:

* Old implementation is now importing experimental TensorStatistic

* Framework-specific implementations are partially updated

I noticed the experimental implementation is missing:

* `statistic_point.py` (StatisticPoint and StatisticPointsContainer)

* `aggregator.py` (StatisticsAggregator)

* `statistics_serializer.py` and `statistics_validator.py`

These components are widely used throughout the codebase (30+ files).

Could you please advice if I should implement these missing components as part of the experimental implementation, or is there a different approach you would recommend?

statistic_point.py (StatisticPoint and StatisticPointsContainer), aggregator.py (StatisticsAggregator), statistics_serializer.py and statistics_validator.py supports statistics from the experimental module. You should replace the following imports on statistics from the experimental module:

{6DE9A02D-8E6A-4EA9-A894-36BE4302C0D0}

{025BD872-9801-405E-965F-04CBB4E6494C}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PT Pull requests that updates NNCF PyTorch NNCF PTQ Pull requests that updates NNCF PTQ NNCF TF Pull requests that updates NNCF TensorFlow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants