Skip to content
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

[PTQ][Torch][Native] Experimental tensor statistics migration #2117

Conversation

daniil-lyakhov
Copy link
Collaborator

@daniil-lyakhov daniil-lyakhov commented Sep 7, 2023

Changes

  • Torch Init range moved to experimental TensorCollector
  • Torch PTQ migrated to experimental TensorCollector
  • Threesigma/MAD aggregators are implemented as experimental tensor collectors for Torch/OV backends
  • no_outlier_map function refactored to not depend on shape of input tensor to make it possible to unify Torch and OV backends as they are using different shape statistics

Reason for changes

  • To use all experimental reducers/aggregators (tensor collectors) in Torch/Torch PTQ backend

Related tickets

109665

Tests

  • Tests are updated
  • Test register_unnamed_inputs
  • Test MedianAbsoluteDeviationAggregator/PrecentileAggregator/PostAggregateHook
  • Test mixed min max TensorCollector creation function

TODO:

  • Confirm that new no_outlier_map performs as good as previous implementation

@daniil-lyakhov daniil-lyakhov requested a review from a team as a code owner September 7, 2023 17:02
@github-actions github-actions bot added NNCF TF Pull requests that updates NNCF TensorFlow NNCF PT Pull requests that updates NNCF PyTorch NNCF Common Pull request that updates NNCF Common experimental NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ labels Sep 7, 2023
@daniil-lyakhov
Copy link
Collaborator Author

@vshampor, @AlexanderDokuchaev, this PR is conflicting with PRs #2032, #2058. We should decide how we will be merging as all three PRs are huge

@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from 58146aa to 681c357 Compare September 7, 2023 17:06
@vshampor vshampor added the API Public API-impacting changes label Sep 7, 2023
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from 681c357 to 79cf566 Compare September 8, 2023 07:15
@vshampor vshampor removed the API Public API-impacting changes label Sep 8, 2023
@daniil-lyakhov daniil-lyakhov changed the title [PTQ][Native][Torch] Refactor tensor statistics to use experimental TensorCollector [PTQ][Torch][Native] Experimental tensor statistics migration Sep 8, 2023
@vshampor
Copy link
Contributor

vshampor commented Sep 8, 2023

@vshampor, @AlexanderDokuchaev, this PR is conflicting with PRs #2032, #2058. We should decide how we will be merging as all three PRs are huge

I closed the #2032, but there will be another one with the algo unification. You should proceed with merging this PR disregarding mine, though - I will rebase my changes accordingly.

@github-actions github-actions bot added the NNCF ONNX Pull requests that updates NNCF ONNX label Sep 8, 2023
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from e1fbd21 to e9aaa44 Compare September 11, 2023 09:26
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #2117 (60c2804) into develop (b16d4a1) will decrease coverage by 0.11%.
Report is 2 commits behind head on develop.
The diff coverage is 39.16%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2117      +/-   ##
===========================================
- Coverage    36.24%   36.13%   -0.11%     
===========================================
  Files          477      480       +3     
  Lines        42570    42998     +428     
===========================================
+ Hits         15428    15539     +111     
- Misses       27142    27459     +317     
Files Coverage Δ
nncf/common/tensor_statistics/collectors.py 77.18% <100.00%> (+1.18%) ⬆️
nncf/common/tensor_statistics/statistics.py 72.72% <100.00%> (+2.13%) ⬆️
nncf/onnx/graph/node_utils.py 97.77% <100.00%> (ø)
...antization/algorithms/bias_correction/algorithm.py 93.88% <ø> (ø)
...quantization/algorithms/bias_correction/backend.py 100.00% <ø> (ø)
...ization/algorithms/bias_correction/onnx_backend.py 97.72% <100.00%> (-0.03%) ⬇️
...antization/algorithms/channel_alignment/backend.py 100.00% <ø> (ø)
...ation/algorithms/fast_bias_correction/algorithm.py 90.83% <ø> (ø)
...ization/algorithms/fast_bias_correction/backend.py 100.00% <ø> (ø)
...on/algorithms/fast_bias_correction/onnx_backend.py 97.64% <100.00%> (-0.03%) ⬇️
... and 25 more

... and 21 files with indirect coverage changes

@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch 2 times, most recently from 884aecb to 53b2067 Compare September 15, 2023 16:00
nncf/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
nncf/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
nncf/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
nncf/experimental/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
tests/torch/quantization/test_range_init.py Outdated Show resolved Hide resolved
tests/torch/quantization/test_range_init.py Outdated Show resolved Hide resolved
tests/torch/quantization/test_range_init.py Show resolved Hide resolved
tests/torch/test_compression_training.py Outdated Show resolved Hide resolved
tests/torch/test_statistics_aggregator.py Show resolved Hide resolved
nncf/experimental/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
nncf/experimental/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
nncf/torch/quantization/init_range.py Outdated Show resolved Hide resolved
nncf/torch/quantization/layers.py Outdated Show resolved Hide resolved
tests/common/experimental/test_reducers_and_aggregators.py Outdated Show resolved Hide resolved
nncf/torch/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
@daniil-lyakhov
Copy link
Collaborator Author

@vshampor, I've renamed percentile and have resolved all relative comments, thanks!

Copy link
Contributor

@vshampor vshampor left a comment

Choose a reason for hiding this comment

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

Better now, still some changes required.

nncf/experimental/common/tensor_statistics/collectors.py Outdated Show resolved Hide resolved
tests/common/experimental/test_reducers_and_aggregators.py Outdated Show resolved Hide resolved
tests/common/experimental/test_reducers_and_aggregators.py Outdated Show resolved Hide resolved
tests/common/experimental/test_statistic_collector.py Outdated Show resolved Hide resolved
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from 43c29f7 to ef99ed4 Compare September 26, 2023 11:47
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch 2 times, most recently from b3fb6b2 to cca8660 Compare September 26, 2023 15:13
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from 8576c17 to 5e859c2 Compare September 27, 2023 13:07
@github-actions github-actions bot added documentation Improvements or additions to documentation dependencies Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM labels Sep 27, 2023
@daniil-lyakhov daniil-lyakhov force-pushed the dl/torch_experimental_statistics branch from 5e859c2 to bf9c611 Compare September 27, 2023 13:09
@github-actions github-actions bot removed documentation Improvements or additions to documentation dependencies Any changes in any dependencies (new dep or its version) should be produced via Change Request on PM labels Sep 27, 2023
Copy link
Contributor

@alexsu52 alexsu52 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alexsu52 alexsu52 left a comment

Choose a reason for hiding this comment

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

Could you provide onnx e2e results?

@daniil-lyakhov
Copy link
Collaborator Author

daniil-lyakhov commented Sep 29, 2023

Could you provide onnx e2e results?

onnx nightly e2e build 455 finished successfully

@alexsu52 alexsu52 merged commit eaddf18 into openvinotoolkit:develop Oct 4, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental NNCF Common Pull request that updates NNCF Common NNCF ONNX Pull requests that updates NNCF ONNX 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.

5 participants