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

[Good First Issue][NNCF][TorchFX]: Test model transformer #2775

Closed
daniil-lyakhov opened this issue Jul 1, 2024 · 16 comments
Closed

[Good First Issue][NNCF][TorchFX]: Test model transformer #2775

daniil-lyakhov opened this issue Jul 1, 2024 · 16 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@daniil-lyakhov
Copy link
Collaborator

daniil-lyakhov commented Jul 1, 2024

Greetings🐱! As a part of #2766 TorchFX PTQ backend support, we are gladly presenting to you following issue

Context

The task is to cover FXModelTransformer by simple unit tests as it done for other backends:
https://github.com/openvinotoolkit/nncf/blob/develop/tests/onnx/test_model_transformer.py

What needs to be done?

Unit tests in file tests/torch/fx/test_model_transformer.py :

  • test_leaf_module_insertion_transformation
  • test_bias_update_transformation
  • test_qdq_insertion_transformation

Example Pull Requests

No response

Resources

Contact points

@daniil-lyakhov

Ticket

141640

@daniil-lyakhov daniil-lyakhov added the good first issue Good for newcomers label Jul 1, 2024
@awayzjj
Copy link

awayzjj commented Jul 9, 2024

.take

Copy link

github-actions bot commented Jul 9, 2024

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@parthrastogicoder
Copy link

.take

Copy link

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

@awayzjj
Copy link

awayzjj commented Jul 14, 2024

@daniil-lyakhov Hi, Could you please push your branch before merging it into the develop branch? This will allow me to develop based on your branch.

Additionally, I noticed the file tests/torch/fx/test_sanity.py. Should I implement the unit tests in tests/torch/fx/test_model_transformer.py instead of tests/torch_fx/test_model_transformer.py?

Thank you!

@daniil-lyakhov
Copy link
Collaborator Author

daniil-lyakhov commented Jul 22, 2024

Hi @awayzjj,

Thank you for your contribution! PR is on review right now and should be merged soon, I'll keep you updated. Yes, please use tests/torch/fx directory, I forgot to update the issue.

Thanks!

@daniil-lyakhov
Copy link
Collaborator Author

Hi @awayzjj, the base PR #2764 was merged, please rebase your cahnges

@awayzjj
Copy link

awayzjj commented Aug 3, 2024

@daniil-lyakhov Hi, I attempted to implement the test_leaf_module_insertion_transformation as follows, but encountered two problems:

import torch
import torch.nn.functional as F
from torch import nn
from torch._export import capture_pre_autograd_graph

from nncf.common.graph.transformations.commands import TargetType
from nncf.common.graph.transformations.layout import TransformationLayout

def test_leaf_module_insertion_transformation():

    class InsertionPointTestModel(nn.Module):
        def __init__(self):
            super().__init__()
            self.conv1 = nn.Conv2d(1, 1, 1, 1)
            self.linear_wts = nn.Parameter(torch.FloatTensor(size=(100, 100)))
            self.conv2 = nn.Conv2d(1, 1, 1, 1)
            self.relu = nn.ReLU()

        def forward(self, input_):
            x = self.conv1(input_)
            x = x.flatten()
            x = nn.functional.linear(x, self.linear_wts)
            x = x.reshape((1, 1, 10, 10))
            x = self.conv2(x)
            x = self.relu(x)
            return x
  

    model = InsertionPointTestModel() 
    with torch.no_grad():
        ex_input = torch.ones([1, 1, 10, 10])
        model.eval()
        exported_model = capture_pre_autograd_graph(model, args=(ex_input,))
    print(exported_model.print_readable())

 
    from nncf.experimental.torch.fx.model_transformer import FXModelTransformer
    from nncf.torch.graph.transformations.commands import PTTargetPoint
    from nncf.experimental.torch.fx.transformations import leaf_module_insertion_transformation_builder
    from nncf.experimental.torch.fx.commands import FXApplyTransformationCommand
    model_transformer = FXModelTransformer(exported_model)

    conv1_node_name = "InsertionPointTestModel/NNCFConv2d[conv1]/conv2d_0"
    target_point = PTTargetPoint(
        target_type=TargetType.OPERATION_WITH_WEIGHTS, target_node_name=conv1_node_name, input_port_id=1
    )
    transformation = leaf_module_insertion_transformation_builder(
        exported_model, [target_point]
    )
    command = FXApplyTransformationCommand(
        transformation
    )
    transformation_layout = TransformationLayout()
    transformation_layout.register(command)
    model_transformer.transform(transformation_layout)
  1. The tests fail with the following exception:
截屏2024-08-03 20 50 38
  1. I have to place the following imports after exported_model = capture_pre_autograd_graph(model, args=(ex_input,))
    from nncf.experimental.torch.fx.model_transformer import FXModelTransformer
    from nncf.torch.graph.transformations.commands import PTTargetPoint
    from nncf.experimental.torch.fx.transformations import leaf_module_insertion_transformation_builder
    from nncf.experimental.torch.fx.commands import FXApplyTransformationCommand

or it raises an error:
截屏2024-08-03 20 54 01

Could you give me some suggestions? Thank you very much!

@rk119
Copy link
Contributor

rk119 commented Aug 4, 2024

@awayzjj you defined the variable conv1_node_name incorrectly. In this case, for example if you define it with conv1_node_name = "conv2d" instead as this is a valid node name, it should work. The node names in the Graph are different for Torch FX backend.

@awayzjj
Copy link

awayzjj commented Aug 4, 2024

@rk119 Thank you so much! Your suggestions does work!

@awayzjj
Copy link

awayzjj commented Aug 26, 2024

@daniil-lyakhov Hi, I've been really busy lately, so I've decided to unassign myself for now. I apologize for any inconvenience this may cause.

@daniil-lyakhov
Copy link
Collaborator Author

@awayzjj, that's ok, thank you for letting us know!

@daniil-lyakhov daniil-lyakhov self-assigned this Aug 27, 2024
@daniil-lyakhov
Copy link
Collaborator Author

PR #2920

@zina-cs
Copy link

zina-cs commented Sep 26, 2024

.take

Copy link

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

@daniil-lyakhov
Copy link
Collaborator Author

daniil-lyakhov commented Sep 26, 2024

@zina-cs, sorry for inconvenience, but this good first issue should have been closed already. Big sorry 🙁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants