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

Patch missed magic functions #2298

Merged

Conversation

AlexanderDokuchaev
Copy link
Collaborator

@AlexanderDokuchaev AlexanderDokuchaev commented Nov 29, 2023

Changes

  • Patch missed magic function
['__ixor__', '__itruediv__', '__rtruediv__', '__abs__', '__ror__', '__ior__', '__rdiv__', '__rxor__', '__iand__', '__neg__',  '__ipow__', '__rpow__', '__invert__', '__rand__']
  • Update functions names for metatypes.
  • New metatype PTNegativeMetatype for -tensor and tensor.neg() operations.

Related tickets

124852

Tests

test_patch_magic_functions
test_op_for_patch_magic_functions

models_hub_test

  • Was 91 failed, 602 passed, after 83 failed, 610 passed

@AlexanderDokuchaev AlexanderDokuchaev requested a review from a team as a code owner November 29, 2023 19:54
@github-actions github-actions bot added the NNCF PT Pull requests that updates NNCF PyTorch label Nov 29, 2023
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Merging #2298 (7828c94) into develop (e17d268) will increase coverage by 0.00%.
Report is 4 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2298   +/-   ##
========================================
  Coverage    90.76%   90.77%           
========================================
  Files          486      487    +1     
  Lines        43783    43806   +23     
========================================
+ Hits         39741    39764   +23     
  Misses        4042     4042           
Flag Coverage Δ
COMMON 15.86% <0.00%> (-0.01%) ⬇️
ONNX 33.86% <100.00%> (+0.01%) ⬆️
OPENVINO 38.72% <100.00%> (+0.01%) ⬆️
TENSORFLOW 30.00% <0.00%> (-0.02%) ⬇️
TORCH 62.69% <100.00%> (+0.01%) ⬆️

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

Files Coverage Δ
nncf/common/graph/patterns/manager.py 96.66% <ø> (ø)
nncf/torch/dynamic_graph/patch_pytorch.py 96.79% <ø> (ø)
nncf/torch/graph/operator_metatypes.py 99.13% <100.00%> (+<0.01%) ⬆️

... and 2 files with indirect coverage changes

@@ -122,35 +122,51 @@ class FunctionsToPatchWithoutTracing:
class MagicFunctionsToPatch:
MAGIC_FUNCTIONS_TO_PATCH = {
NamespaceTarget.TORCH_TENSOR: [
"__abs__",
Copy link
Contributor

Choose a reason for hiding this comment

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

Metatypes should be defined for all new operations and added support in NNCF algorithms.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added

@daniil-lyakhov
Copy link
Collaborator

@AlexanderDokuchaev, is it possible to add some unit tests for this change?

@github-actions github-actions bot added the NNCF Common Pull request that updates NNCF Common label Dec 5, 2023
@AlexanderDokuchaev AlexanderDokuchaev force-pushed the ad/fix_trace_graph branch 2 times, most recently from 6b6ac7d to 7828c94 Compare December 5, 2023 13:41
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.

👍

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

@alexsu52 alexsu52 merged commit fa67e00 into openvinotoolkit:develop Dec 6, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NNCF Common Pull request that updates NNCF Common NNCF PT Pull requests that updates NNCF PyTorch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants