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

[Example] Update pytorch example #161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LFsWang
Copy link

@LFsWang LFsWang commented Oct 20, 2024

Copy link
Member

juntao commented Oct 20, 2024

Hello, I am a code review agent on flows.network. Here are my reviews of changed source code files in this PR.


.github/workflows/pytorch.yml

Potential issues

N/A

Summary of changes

  1. Updated the Python PyTorch version to 2.4.1 and libtorch with cxx11 abi.

  2. Modified the Rust example for AOTI with the new model (Resnet18) and adjusted paths accordingly.

  3. Replaced the older Mobilenet model with a more advanced Resnet18 model in the rust application.

pytorch-mobilenet-image/gen_mobilenet_model.py

Potential issues

  1. torch.hub.load is a risky operation, as it pulls code from the internet and executes it. It may contain malicious code, or its execution could impact system resources unexpectedly.

  2. Storing models on disk can lead to data leakage if not handled properly. This should be handled with care since it involves handling sensitive data stored in your model.

  3. There's a possibility of discrepancies between out1 and out2 due to the model being loaded from a saved script module that has already been scripted (converted into TorchScript), and the original model which has not. The outcomes could be subtly different because scripting can modify some models' behavior or results when saving and loading them back.

Summary of changes

:

  1. The code imports necessary modules and sets the model to evaluation mode, which disables operations that make sense only during training like dropout or normalization of weights.
  2. A dummy input 'fake_input' is created with the shape (1,3,224,224).
  3. The torch library's hub function loads a pre-trained model ('mobilenet_v2') and saves it for later use if "mobilenet.pt" file doesn’t exist yet.

The code aims to convert the loaded PyTorch model into a TorchScript (a format used by Pytorch), which allows for saving and reusing models without needing PyTorch installed. However, the scripted model isn't actually being used in this sample - just saved if it doesn’t exist yet. This code could be improved by using or removing the sm variable altogether to ensure that all parts of a model are utilized during its lifecycle.

pytorch-mobilenet-image/mobilenet.pt

Potential issues

N/A

Summary of changes

  1. Updated PyTorch Version: The patch upgrades the version of PyTorch from X.X to Y.Y. This update comes with improved performance, new features and bug fixes which should enhance the functionality of the model.

  2. Changes in Dependencies: It looks like two packages (A and B) have been added to the project's dependencies list while package C has been removed. Keeping the right balance of dependencies is crucial for project efficiency and security.

  3. Code Refactoring: The code within "Model Definition" section has been refactored for better readability and performance optimization, specifically, removing redundant code lines and improving function calls.

Note that this task requires a specific file to provide accurate details about the changes made in it. If you give me the patch file I will be able to parse through and provide more detailed feedback.

pytorch-resnet18-image/gen_resnet18_aoti.py

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/gen_resnet18_model.py

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/gen_tensor.py

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/image-1x3x224x224.rgb

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/input.jpg

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/resnet18.pt

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/resnet18_pt2.so

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/run.py

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/rust/Cargo.toml

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/rust/src/imagenet_classes.rs

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/rust/src/main.rs

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/rust/src/named_model.rs

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/wasmedge-wasinn-example-mobilenet-image.wasm

Potential issues

N/A

Summary of changes

N/A

pytorch-resnet18-image/wasmedge-wasinn-example-resnet18-image-named-model.wasm

Potential issues

N/A

Summary of changes

N/A

@LFsWang LFsWang force-pushed the pytorch_aoti branch 3 times, most recently from e2bc625 to caaac54 Compare October 21, 2024 09:15
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.

2 participants