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

[WC] Fix ratio_defining_params #2653

Merged

Conversation

l-bat
Copy link
Collaborator

@l-bat l-bat commented Apr 24, 2024

Changes

Reason for changes

Fix bug in _get_ratio_defining_params method

Tests

test_shared_gather_all_layers

@l-bat l-bat requested a review from a team as a code owner April 24, 2024 20:53
@github-actions github-actions bot added NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ labels Apr 24, 2024
Copy link

codecov bot commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 29.87%. Comparing base (21bd852) to head (15afc14).
Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop    #2653       +/-   ##
============================================
- Coverage    62.02%   29.87%   -32.16%     
============================================
  Files          495      495               
  Lines        45973    45971        -2     
============================================
- Hits         28516    13735    -14781     
- Misses       17457    32236    +14779     
Files Coverage Δ
...ization/algorithms/weight_compression/algorithm.py 0.00% <0.00%> (-95.22%) ⬇️

... and 264 files with indirect coverage changes

Flag Coverage Δ
COMMON ?
ONNX ?
OPENVINO ?
TENSORFLOW 29.87% <0.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
common 76.35% <ø> (-12.10%) ⬇️
torch 0.01% <ø> (-32.84%) ⬇️
tensorflow 93.74% <ø> (ø)
onnx 0.00% <ø> (-93.07%) ⬇️
openvino 0.00% <ø> (-94.25%) ⬇️
ptq 15.19% <0.00%> (-65.48%) ⬇️

@l-bat l-bat requested a review from ljaljushkin April 25, 2024 07:39
ratio_defining_params = list(
filter(
lambda wp: wp.node_with_weight.metatype in self._backend_entity.matmul_metatypes,
all_weight_params,
)
)

# The last MatMul layer is quantized to 4-bits if all_layers=True or if the layer is shared
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment looks wrong.
if the layer is shared means that it has the same precision as embedding, and it's 4bit only with all_layers

Copy link
Contributor

@ljaljushkin ljaljushkin left a comment

Choose a reason for hiding this comment

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

LGTM, would be nice to correct comment, but it's minor

@l-bat l-bat force-pushed the lt/fix_ratio_defining_params branch from bbc7f71 to 15afc14 Compare May 13, 2024 14:43
@l-bat l-bat requested a review from ljaljushkin May 14, 2024 12:44
@ljaljushkin ljaljushkin merged commit d94c564 into openvinotoolkit:develop May 14, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NNCF OpenVINO Pull requests that updates NNCF OpenVINO NNCF PTQ Pull requests that updates NNCF PTQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants