[TorchAO] [OVQuantizer] Update Torch AO Dependency and Refactor Executorch Tests#3842
[TorchAO] [OVQuantizer] Update Torch AO Dependency and Refactor Executorch Tests#3842anzr299 wants to merge 59 commits intoopenvinotoolkit:developfrom
Conversation
examples/post_training_quantization/torch_fx/resnet18/requirements.txt
Outdated
Show resolved
Hide resolved
| from nncf.quantization.range_estimator import RangeEstimatorParameters | ||
|
|
||
|
|
||
| def _is_openvino_quantizer_instance(obj) -> bool: |
There was a problem hiding this comment.
Looks like WA for mistakes in https://github.com/pytorch/executorch/blame/main/backends/openvino/quantizer/quantizer.py
It's really bad import tree.
Need some time...
There was a problem hiding this comment.
There is a dependency and ownership issue around the quantization entry point that needs clarification.
Currently:
- quantize_pt2e depends on OpenVINOQuantizer (from executorch).
- quantize_model (also in executorch) depends on quantize_pt2e.
- OpenVINOQuantizer and quantize_model are defined in the same file.
As a result, the effective call chain looks like this: executorch → nncf → executorch
This creates a circular dependency and makes it unclear what the intended public entry point is for OpenVINO PT2E quantization. Since everything lives in a single executorch file, it’s not obvious:
- which function users are expected to call directly
- which layer “owns” the quantization workflow (executorch or NNCF).
So my current suggest to dont block this PR:
- Create ticket on it, add TODO here.
- Refactor openvino backend files in executorch
- Finalize quantize_pt2e
| """ | ||
| try: | ||
| from executorch.backends.openvino.quantizer.quantizer import OpenVINOQuantizer | ||
| except ModuleNotFoundError as err: |
There was a problem hiding this comment.
But _is_openvino_quantizer_instance function is useless.
In case to avoid circular import just move import inside a function.
And it's not safely check cause raise error. For optional dependencies it's should looks like
try:
from executorch.backends.openvino.quantizer.quantizer import OpenVINOQuantizer
return isinstance(obj, OpenVINOQuantizer)
except ImportError:
return False
And better use ImportError for checks like this, because for old version of executorch will raised ImportError.
ModuleNotFoundError is subclass of ImportError.
There was a problem hiding this comment.
oh okay, I will change it
tests/executorch/requirements.txt
Outdated
| pytest-xdist==3.5.0 | ||
| pytest-forked==1.6.0 | ||
| pytest-split==0.9.0 | ||
| ipython==8.37.0 No newline at end of file |
There was a problem hiding this comment.
ipython is not requared.
Use fastprogress==1.0.5 as it in constraints.txt #3815
| @@ -0,0 +1,1038 @@ | |||
| strict digraph { | |||
There was a problem hiding this comment.
Differ from old tests/torch/data/fx/OpenVINOQuantizer/mobilenet_v3_small.dot
Is it expected? What has changed?
There was a problem hiding this comment.
It was a bug, it is changed ot be the same now
|
@anzr299 What version of executorch is required to run tests? |
Executorch 1.0.1 for PTQ tests. |
| @@ -1,5 +1,6 @@ | |||
| tensorboard==2.13.0 | |||
| torch==2.9.0 | |||
| torchao==0.14.0 | |||
There was a problem hiding this comment.
Why torch example require torchao?
It's not use anything from it.
torchao and executorch should not be required dependency for TORCH backend
There was a problem hiding this comment.
It was importing torchao in src/nncf/torch/quantization/strip.py. Earlier this was torch.ao so it was hidden
https://github.com/openvinotoolkit/nncf/actions/runs/21142246987/job/60798968378
Perhaps I can make it lazy import inside the convert_to_torch_fakequantizer function
tests/post_training/requirements.txt
Outdated
| optimum-onnx==0.1.0 | ||
| optimum==2.1.0 | ||
| scikit-learn>=1.2.2,<=1.5.0 | ||
| scikit-learn>=1.2.2,<=1.7.1 |
There was a problem hiding this comment.
Set strict version of used package (get version from test job)
| preset = quantizer_kwargs.pop("preset", None) | ||
| model_type = quantizer_kwargs.pop("model_type", None) | ||
|
|
||
| # This logic is copied and inverted from OVQuantizer code |
There was a problem hiding this comment.
Why is it here?
Please use model_scope.py to setup compression arguments without any hidden behavior.
| from nncf.quantization.range_estimator import RangeEstimatorParameters | ||
|
|
||
|
|
||
| def _is_openvino_quantizer_instance(obj) -> bool: |
There was a problem hiding this comment.
There is a dependency and ownership issue around the quantization entry point that needs clarification.
Currently:
- quantize_pt2e depends on OpenVINOQuantizer (from executorch).
- quantize_model (also in executorch) depends on quantize_pt2e.
- OpenVINOQuantizer and quantize_model are defined in the same file.
As a result, the effective call chain looks like this: executorch → nncf → executorch
This creates a circular dependency and makes it unclear what the intended public entry point is for OpenVINO PT2E quantization. Since everything lives in a single executorch file, it’s not obvious:
- which function users are expected to call directly
- which layer “owns” the quantization workflow (executorch or NNCF).
So my current suggest to dont block this PR:
- Create ticket on it, add TODO here.
- Refactor openvino backend files in executorch
- Finalize quantize_pt2e
|
Agreed to merge it after the CF to mitigate any risks with a new dependency. @anzr299 @AlexanderDokuchaev |
| # Pytorch | ||
| torch==2.9.0 | ||
| torchvision==0.24.0 | ||
| torchao==0.14.0 |
There was a problem hiding this comment.
Aamir, as we discussed it with Alexander Suslov, the main idea was to get rid of torch.ao and do not use any external dependencies (like torchao)
The task is to remove torch.ao imports and replace them with code in the nncf codebase, no external dependencies
Changes
Include tests for executorch, OVQuantizer
Remove OVQuantizer from NNCF
Move the tests in appropriate directory
Reason for changes
Migrate from torch.ao -> torchao
Notable differences in torch.ao and torchao implementation which affect test references:
torchaoimplementation it keeps the same name as the node being replaced with.https://github.com/pytorch/executorch/tree/main/backends/xnnpack/quantizerwhich now explicitly quantizes the conv_transpose2D operator and its weights.Reference graphs changed due to above:
Related tickets
CVS-176783
Tests
PTQ Conformance - PTQ-789 - Pass
Examples - https://github.com/openvinotoolkit/nncf/actions/runs/21204888584 - Pass
WC conformance - https://github.com/openvinotoolkit/nncf/actions/runs/21204893707 - Pass
pytest tests/executorch results:

All tests in tests/executorch pass