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

Transforming Models from PyTorch/Tensorflow to POET #9

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

arnavsinghvi11
Copy link
Collaborator

@arnavsinghvi11 arnavsinghvi11 commented Feb 22, 2023

This PR is the first step towards translating models defined in Torch to Tensorflow transparently. We define network transformation functions which are then composed together for converting PyTorch and Tensorflow network layers to POET computation layers.

Tested for ResNet18, ResNet50, and VGG16.

@arnavsinghvi11
Copy link
Collaborator Author

created network transformation function for converting TensorFlow network layers to POET computation layers

testing is done for ResNet model

@arnavsinghvi11
Copy link
Collaborator Author

created network transformation function for converting PyTorch model graph to graph with POET computation layer nodes

testing is done for ResNet18, ResNet50

Copy link
Owner

@ShishirPatil ShishirPatil left a comment

Choose a reason for hiding this comment

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

Hey @arnavsinghvi11 great job on the PR! It looks great in general. Left some minor comments. Let me know if you have any questions. Thanks again for this!!



# transforms input model's graph to output graph with POET layer nodes
def graph_transform(traced):
Copy link
Owner

Choose a reason for hiding this comment

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

To improve readability can we use type hints? As type torch.nn as input and as type poet.DNNLayer as expected output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The function actually takes in the graph of a torch model and outputs a recompiled version of the graph with poet.DNNLayers. I printed the type of this graph which is: torch.fx.graph_module.GraphModule.new..GraphModuleImpl so do you still recommend including this as a type hint or would that stray away from readability?

# transforms input model's graph to output graph with POET layer nodes
def graph_transform(traced):
for n in traced.graph.nodes:
if "<built-in function" in str(n.target):
Copy link
Owner

Choose a reason for hiding this comment

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

  1. Add a line to clarify why we ignore built-in functions?
  2. If the torch layer is not part of POET's vocab, how do we handle it? One suggestion - a) we inform the user and if they confirm, b) we ignore the layer and continue given the input/output dimensionality is respected. Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed, would adding a print statement informing the layer is not supported by POET and continuing looping through suffice?


# transforms ResNet Model graph into POET layers nodes

# #Resnet18 model transformation - https://github.com/pytorch/vision/blob/main/torchvision/models/resnet.py, commit: 7dc5e5bd60b55eb4e6ea5c1265d6dc7b17d2e917
Copy link
Owner

Choose a reason for hiding this comment

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

Is this commented for a reason? Good to have the tests in.

num_classes = 10
layers = [InputLayer((batch_size, *input_shape))]

# comment out to output transformations for different ResNet models
Copy link
Owner

Choose a reason for hiding this comment

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

Slightly confused by the comment message.

from poet.power_computation import InputLayer
import tensorflow as tf

# transforms VGG Model network layers into POET computation layers
Copy link
Owner

Choose a reason for hiding this comment

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

The comment reads VGG Model but I think the ResNet model is being constructed here?

from poet.power_computation_transformer import QueryKeyValueMatrix, QKTMatrix, QKTVMatrix
import torch.nn as nn
import torchvision.models
from torchvision.models.resnet import BasicBlock, Bottleneck
Copy link
Owner

Choose a reason for hiding this comment

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

We say network transform, but this is mostly for Resnet+common layers? I think we should have a better way to organize this. I don't know exactly how - open to suggestions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the goal was to extend this for other models like BERT which I didn't fully support in my implementation yet. I can comment out this import for now.

@ShishirPatil ShishirPatil changed the title network transformation function & testing for PyTorch to POET Transforming Models from PyTorch/Tensorflow to POET Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants