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

[Torch FX] Add Special FX Metatype in Torch Operator Metatypes #2976

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

anzr299
Copy link
Contributor

@anzr299 anzr299 commented Sep 19, 2024

Changes

  1. Create a new registry for FX metatypes
  2. Create new metatype FXEmbeddingMetatype for torch FX embedding metatype

Reason for changes

The current torch embedding metatype, which maps to torch.nn.functional.embedding has weight node on port 1, but when using torch.nn.Embedding with torch fx, since it has weight on port 0, the metatype should be defined accordingly.

@anzr299 anzr299 requested a review from a team as a code owner September 19, 2024 11:22
@github-actions github-actions bot added NNCF PT Pull requests that updates NNCF PyTorch experimental labels Sep 19, 2024
@@ -120,15 +135,15 @@ def create_nncf_graph(model: torch.fx.GraphModule) -> PTNNCFGraph:

for source_node in model.graph.nodes:
node_type, node_metatype = GraphConverter._get_node_type_and_metatype(source_node, model)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you suggest how to take function namespace into account when defining metatype?
cc' @daniil-lyakhov @AlexanderDokuchaev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I will check for possible ways to map with function namespaces as well.

@daniil-lyakhov
Copy link
Collaborator

Apparently covered by #2891

@anzr299
Copy link
Contributor Author

anzr299 commented Sep 30, 2024

Apparently covered by #2891

It needs some additional work as @alexsu52 mentioned, mapping the node metatype to the function names.

@daniil-lyakhov
Copy link
Collaborator

daniil-lyakhov commented Sep 30, 2024

@anzr299, please change the description of the PR and reopen it then. Please reflect it in #2938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental NNCF PT Pull requests that updates NNCF PyTorch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants