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

[Conformance] TorchFX/OV backends Alignment #2996

Merged

Conversation

daniil-lyakhov
Copy link
Collaborator

@daniil-lyakhov daniil-lyakhov commented Sep 30, 2024

Changes

  • Constant folding is applied to all TorchFX models before the quantization
  • Some torchvision models (swin_v2_s, vit_16_b) are exported by torch.export.export before ov conversation
  • Moc transformations are applied to openvino compressed models after the compression

After the #2984

  • Fixed _compress_qdq_constant_transformation for per tensor case

Reason for changes

  • To align TorchFX/OV quantized models

Related tickets

#2766

Tests

post_training_quantization/504/ is finished successfully

@github-actions github-actions bot added experimental NNCF PTQ Pull requests that updates NNCF PTQ labels Sep 30, 2024
@daniil-lyakhov daniil-lyakhov force-pushed the dl/fx/constant_folding branch 2 times, most recently from 61e48e7 to 455a23d Compare October 4, 2024 16:30
@daniil-lyakhov daniil-lyakhov changed the title [Experimental][Conformance][FX] Constant folding [Conformance] TorchFX/OV backends Alignment Oct 4, 2024
@daniil-lyakhov daniil-lyakhov marked this pull request as ready for review October 8, 2024 15:42
@daniil-lyakhov daniil-lyakhov requested a review from a team as a code owner October 8, 2024 15:42
@github-actions github-actions bot added the NNCF PT Pull requests that updates NNCF PyTorch label Oct 11, 2024
@alexsu52
Copy link
Contributor

Please rerun post_training_quantization build.

@daniil-lyakhov
Copy link
Collaborator Author

Please rerun post_training_quantization build.

/post_training_quantization/504/ is finished successfully

@AlexanderDokuchaev
Copy link
Collaborator

Please add unit tests for constant folding and docstrings.

@daniil-lyakhov
Copy link
Collaborator Author

Please add unit tests for constant folding and docstrings.

tested by test_models by swin_v2_s and vit_b_16 models. The same test approach is done for other transformation.
If you think we definitely need a unit test - let's create an issue for that

@AlexanderDokuchaev
Copy link
Collaborator

@alexsu52 i will not review PR that adding transformation that will applied for any model in nncf.quantize without unit tests and any documentation.

@daniil-lyakhov
Copy link
Collaborator Author

@AlexanderDokuchaev, please take a look

@anzr299
Copy link
Contributor

anzr299 commented Oct 21, 2024

In the case where the user gives a model with already inserted Quantize-Dequantize or Quantize-random_nodes-Dequantize, shouldn't it be ignored by constant folding?
cc @alexsu52

@daniil-lyakhov
Copy link
Collaborator Author

@anzr299, I fixed _compress_qdq_constant_transformation function in the per tensor case and tested it by test_compress_post_quantize_transformation, take a look

@alexsu52 alexsu52 requested a review from anzr299 October 22, 2024 10:50
weight_node = get_const(nodes_map["weight"])
scale_node = get_const(nodes_map["scale"])
zp_node = get_const(nodes_map["zero_point"])
axis = nodes_map["axis"]
axis = nodes_map.get("axis")
Copy link
Contributor

@anzr299 anzr299 Oct 22, 2024

Choose a reason for hiding this comment

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

I think we can use the same function for axis too.

Suggested change
axis = nodes_map.get("axis")
axis = get_const(nodes_map.get("axis"))

Copy link
Collaborator Author

@daniil-lyakhov daniil-lyakhov Oct 22, 2024

Choose a reason for hiding this comment

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

Nope, per tensor case has no "axis" key, [] raises key error. Intended axis value for per tensor case - None. .get returns None https://docs.python.org/3/library/stdtypes.html#dict.get

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry about that, I have updated the suggestion. I meant to say that we can pass this also to the get_const funciton to keep it the same as others

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@anzr299
Copy link
Contributor

anzr299 commented Oct 22, 2024

@anzr299, I fixed _compress_qdq_constant_transformation function in the per tensor case and tested it by test_compress_post_quantize_transformation, take a look

Oh, yes that implementation did not consider per_tensor quantization. Thank you for pointing it out!

nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
nncf/experimental/torch/fx/constant_folding.py Outdated Show resolved Hide resolved
@alexsu52 alexsu52 merged commit b280eb7 into openvinotoolkit:develop Oct 30, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Freeze 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.

4 participants