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

Switch PyTorch PTQ to common implementation #2227

Conversation

AlexanderDokuchaev
Copy link
Collaborator

@AlexanderDokuchaev AlexanderDokuchaev commented Oct 26, 2023

Changes

  • Switch nncf.quantize to post training implementation for PT backend
  • Removed OLD_TORCH backend from test_quantize_conformance.py
  • Add support of dict as output of calibration dataset

Related tickets

119910

@github-actions github-actions bot added NNCF PT Pull requests that updates NNCF PyTorch experimental NNCF PTQ Pull requests that updates NNCF PTQ labels Oct 26, 2023
@openvino-nncf-ci openvino-nncf-ci added the API Public API-impacting changes label Oct 26, 2023
@AlexanderDokuchaev AlexanderDokuchaev marked this pull request as ready for review October 26, 2023 23:50
@AlexanderDokuchaev AlexanderDokuchaev requested a review from a team as a code owner October 26, 2023 23:50
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #2227 (4a82dcb) into develop (76bb1f0) will increase coverage by 53.21%.
Report is 7 commits behind head on develop.
The diff coverage is 97.82%.

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #2227       +/-   ##
============================================
+ Coverage    36.40%   89.61%   +53.21%     
============================================
  Files          486      485        -1     
  Lines        43508    43421       -87     
============================================
+ Hits         15838    38912    +23074     
+ Misses       27670     4509    -23161     
Flag Coverage Δ
ONNX 33.77% <0.00%> (?)
OPENVINO 38.42% <0.00%> (?)
TENSORFLOW 30.10% <0.00%> (?)
TORCH 62.55% <97.82%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
nncf/quantization/quantize_model.py 84.28% <ø> (+37.80%) ⬆️
nncf/torch/engine.py 100.00% <100.00%> (+100.00%) ⬆️
nncf/torch/quantization/quantize_model.py 95.71% <97.50%> (+95.71%) ⬆️

... and 365 files with indirect coverage changes

@AlexanderDokuchaev
Copy link
Collaborator Author

post_training_quantization/193/

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
Collaborator

@daniil-lyakhov daniil-lyakhov left a comment

Choose a reason for hiding this comment

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

Minor

nncf/torch/engine.py Outdated Show resolved Hide resolved
nncf/torch/quantization/quantize_model.py Outdated Show resolved Hide resolved
@MaximProshin
Copy link
Collaborator

@vshampor , please state explicitly if there are any remaining comments from you that should be applied or you're ok to merge it.

@vshampor
Copy link
Contributor

vshampor commented Nov 2, 2023

I'm not blocking it, but the change will have to be aligned with #2196 in one way or another.

@KodiaqQ KodiaqQ merged commit f8bdbc7 into openvinotoolkit:develop Nov 2, 2023
6 checks passed
KodiaqQ pushed a commit that referenced this pull request Nov 30, 2023
### Changes

- Update torch examples, added argument `--export-model-path` instead of
`--to-onnx` that select output type by suffix '.xml' or '.onnx'.
- Update tests to use Command to run examples.
- Remove tests of q_dq export path.
- Remove code to make html report, now metrics dumps in `results.csv`.
- Added "build url" in `results.csv`.
- Actualize reference metrics. 
- Move normalization of input to accuracy cheker configs.
- Use `target_ov` and `target_pt` to define reference metrics for
backends.
- Skip test `unet_mapillary_int8` and
`unet_mapillary_magnitude_sparsity_int8` models by 123448.
- Skip `resnet18_imagenet_binarization_dorefa` model by 22543.

Updates in CI:
- Update script to report `final e2e_result.html` for e2e tests.
- Now if any test falls trigger_job will be fails.

After merge requires update ci-pipelines.

### Related tickets

117885

### Tests

tests/torch/test_sota_checkpoints.py

### TODO
- update metrics after merge
#2227
- check metrics after #2211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Public API-impacting changes experimental NNCF PT Pull requests that updates NNCF PyTorch NNCF PTQ Pull requests that updates NNCF PTQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants