[Torch FX] Map Namespace Names to Metatype #3237

anzr299 · 2025-02-03T11:46:02Z

Changes

Created a new dictionary in operator metatypes registry to maintain mapping of namespace and function name to metatype.

Reason for changes

For a more accurate retrieval of operation metatype.

Tests

original graph tests and metatypes were used to check the correctness of the change.

nncf/common/graph/operator_metatypes.py

nncf/experimental/torch/fx/nncf_graph_builder.py

nncf/common/graph/operator_metatypes.py

nncf/experimental/torch/fx/nncf_graph_builder.py

remove unnecesary print remove unnecesary FX metatype Registry pre commit fix

alexsu52

LGTM

daniil-lyakhov · 2025-02-05T09:52:22Z

@anzr299, please fix the pre-commit:
KeyError: 'PTInputNoopMetatype is already registered in input_noop_metatypes'

alexsu52 · 2025-02-13T04:03:48Z

#3258 (comment)

cc' @AlexanderDokuchaev

daniil-lyakhov · 2025-02-26T12:58:31Z

A test case have caught the issue with an embedding model here ynimmaga/executorch#26
@anzr299, please prioritize this fix

… metatypes. Update reference graphs

AlexanderDokuchaev · 2025-03-10T13:30:25Z

nncf/common/graph/operator_metatypes.py

@@ -78,6 +78,7 @@ def __init__(self, name: str):
        """
        super().__init__(name)
        self._op_name_to_op_meta_dict: Dict[str, Type[OperatorMetatype]] = {}
+        self._func_name_to_op_meta_dict: Dict[str, Type[OperatorMetatype]] = {}


AlexanderDokuchaev · 2025-03-10T13:31:42Z

nncf/experimental/torch/fx/nncf_graph_builder.py

@@ -205,4 +203,4 @@ def get_edge_params(
            nncf_logger.debug(f"Edge shape between {source_node.name} and {dist_node.name} is unknown.")

        input_port_id = dist_node.all_input_nodes.index(source_node)
-        return input_port_id, output_port_id, tensor_shape
+        return input_port_id, output_port_id, tensor_shape, tensor_dtype


Need to update return annotation

AlexanderDokuchaev · 2025-03-10T18:11:57Z

nncf/torch/graph/operator_metatypes.py

 from nncf.torch.dynamic_graph.graph import DynamicGraph
 from nncf.torch.dynamic_graph.structs import NamespaceTarget

 ModuleAttributes = TypeVar("ModuleAttributes", bound=BaseLayerAttributes)

 PT_OPERATOR_METATYPES = OperatorMetatypeRegistry("operator_metatypes")
 FX_OPERATOR_METATYPES = OperatorMetatypeRegistry("operator_metatypes")
+PT2_OPERATOR_METATYPES = OperatorMetatypeRegistry("operator_metatypes")


No need to add extra registry use only one, but use full operation name as op_name

AlexanderDokuchaev · 2025-03-10T18:31:11Z

nncf/experimental/torch2/function_hook/graph/graph_utils.py

+    @property
+    def func_namespace(self) -> str:
+        if self.func.__qualname__.split(".")[0] == "TensorBase":
+            return f"torch.tensor.{str(self.func.__name__)}"


no need to use str within f-string

oh, I missed that when changing from an older implementation. Thank you very much!

AlexanderDokuchaev · 2025-03-10T18:50:46Z

nncf/experimental/torch2/function_hook/graph/graph_utils.py

@@ -82,6 +82,14 @@ class FunctionMeta:
    def func_name(self) -> str:
        return self.func.__name__

+    @property
+    def func_namespace(self) -> str:


Change func_name property, there is need only one way to get name

AlexanderDokuchaev · 2025-03-10T18:52:27Z

nncf/experimental/torch2/function_hook/graph/graph_utils.py

+        if self.func.__qualname__.split(".")[0] == "TensorBase":
+            return f"torch.tensor.{str(self.func.__name__)}"
+        elif self.func.__qualname__ == self.func.__name__:
+            return f"torch.nn.functional.{str(self.func.__name__)}"


It's not correctly works for function from torchvision like deform_conv2d or symmetric_quantize in nncf
So better use f"{func.__module__}.{func.__name__}" as op_name for functions and func.__qualname__ for methods.

And using NamespaceTarget in definition of metatypes is actual only for patched functions, for FX and TorchFunctionMode tracing is it should not be used.

@alexsu52

Sorry I did not understand the suggestion "f"{func.module}.{func.name}" as op_name for functions and func.qualname for methods" do you suggest using these directly? because for example torch.tensor.to does not have a module but qualname returns TensorBase.to

yes,
torch.tensor.to is method and can not be call as torch.tensor.to and methods is not contain __module__
It can used only for instance of Tensor torch.tensor([1]).to.__qualname__
So i have suggest to use qualname for it

Okay, so I will modify the metatype definition to be tensorbase.to instead of torch.tensor.to so that it can match with the correct metatype? or do I create a fake tensor and call Tensor([1]).to.__qualname__ on it?

review fixes

anzr299 requested a review from a team as a code owner February 3, 2025 11:46

github-actions bot added NNCF PT Pull requests that updates NNCF PyTorch NNCF Common Pull request that updates NNCF Common experimental labels Feb 3, 2025

alexsu52 reviewed Feb 4, 2025

View reviewed changes

nncf/common/graph/operator_metatypes.py Outdated Show resolved Hide resolved

nncf/experimental/torch/fx/nncf_graph_builder.py Outdated Show resolved Hide resolved

alexsu52 requested a review from daniil-lyakhov February 4, 2025 13:10

alexsu52 self-assigned this Feb 4, 2025

daniil-lyakhov requested changes Feb 4, 2025

View reviewed changes

nncf/common/graph/operator_metatypes.py Outdated Show resolved Hide resolved

nncf/experimental/torch/fx/nncf_graph_builder.py Outdated Show resolved Hide resolved

anzr299 force-pushed the fx/metatypes branch from 4fb49f8 to ecae4d8 Compare February 4, 2025 15:38

comment changes

98cb4dc

remove unnecesary print remove unnecesary FX metatype Registry pre commit fix

anzr299 force-pushed the fx/metatypes branch from 062b485 to 98cb4dc Compare February 4, 2025 16:24

anzr299 added 2 commits February 4, 2025 20:28

update operator metatypes

5991aef

revert breaking changes

44103b4

alexsu52 approved these changes Feb 5, 2025

View reviewed changes

anzr299 requested a review from daniil-lyakhov February 5, 2025 07:48

Update operator_metatypes.py

22c5e51

anzr299 added 2 commits February 6, 2025 11:25

fix aten op error

d53b445

fix

de1361c

alexsu52 mentioned this pull request Feb 7, 2025

[PT2] FBC #3258

Merged

daniil-lyakhov mentioned this pull request Feb 26, 2025

[OpenVINOQuantizer] Tests for the OpenVINOQuantizer ynimmaga/executorch#26

Draft

anzr299 added 11 commits February 28, 2025 00:08

Merge branch 'openvinotoolkit:develop' into fx/metatypes

215a529

update synthetic transformer reference graphs and reference values

a36c4e3

update get_all_aliases method

4ec56be

add suport for edge case for int input of embedding node

a0de15d

Merge branch 'openvinotoolkit:develop' into fx/metatypes

fb49a78

fix error

4e716c5

add PT2 metatype mapping from namespace

56dfdda

pre commit fix

38e825a

add pt2 operator metatype registry to known registries in PT operator…

f863b74

… metatypes. Update reference graphs

pre commit fix

2bdc08f

update reference graphs; fix minor debugging leftover

441dde9

anzr299 requested a review from AlexanderDokuchaev March 5, 2025 02:29

anzr299 added 4 commits March 6, 2025 10:34

Merge branch 'develop' into fx/metatypes

cbda8b6

Merge branch 'openvinotoolkit:develop' into fx/metatypes

8a4e382

update graph builder for dynamic shapes

ab71b81

update reference graphs

97dc74a

AlexanderDokuchaev requested changes Mar 10, 2025

View reviewed changes

Remove Extra Registry;

8fb74e8

review fixes

github-actions bot added API Public API-impacting changes and removed NNCF Common Pull request that updates NNCF Common labels Mar 10, 2025

update metatypes

a90375c

github-actions bot removed the API Public API-impacting changes label Mar 11, 2025

anzr299 added 5 commits March 11, 2025 11:55

Merge branch 'openvinotoolkit:develop' into fx/metatypes

11565c2

update reference graphs and metatype for convtranspose2d

ffddec7

revert reference graph changes

f7fd41c

update a wrong reference file

3f2c342

update operator metatype mapping

9acc6ae

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Torch FX] Map Namespace Names to Metatype #3237

[Torch FX] Map Namespace Names to Metatype #3237

anzr299 commented Feb 3, 2025

alexsu52 left a comment

daniil-lyakhov commented Feb 5, 2025

alexsu52 commented Feb 13, 2025 •

edited

Loading

daniil-lyakhov commented Feb 26, 2025

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 10, 2025

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 10, 2025

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 10, 2025 •

edited

Loading

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 10, 2025

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 10, 2025

AlexanderDokuchaev Mar 10, 2025

anzr299 Mar 11, 2025

AlexanderDokuchaev Mar 11, 2025

anzr299 Mar 11, 2025

[Torch FX] Map Namespace Names to Metatype #3237

Are you sure you want to change the base?

[Torch FX] Map Namespace Names to Metatype #3237

Conversation

anzr299 commented Feb 3, 2025

Changes

Reason for changes

Tests

alexsu52 left a comment

Choose a reason for hiding this comment

daniil-lyakhov commented Feb 5, 2025

alexsu52 commented Feb 13, 2025 • edited Loading

daniil-lyakhov commented Feb 26, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anzr299 Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexsu52 commented Feb 13, 2025 •

edited

Loading

anzr299 Mar 10, 2025 •

edited

Loading