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

Update NN related content #154

Merged
merged 16 commits into from
Aug 21, 2023
Merged

Update NN related content #154

merged 16 commits into from
Aug 21, 2023

Conversation

juntao
Copy link
Member

@juntao juntao commented Aug 20, 2023

Explanation

Cover complete inference functions for mediapipe and other popular models.

Improve documentation on backends.

Signed-off-by: Michael Yuan <michael@secondstate.io>
Copy link
Collaborator

alabulei1 commented Aug 20, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Overall, the GitHub Pull Request titled "Update NN related content" includes a variety of changes to different documentation files related to neural networks (NN) and AI inference in Rust and WasmEdge.

The individual summaries highlight potential issues and errors, as well as the most significant findings. Some potential problems identified are:

  • The removal of significant content without sufficient explanation or reasoning.
  • The removal of code snippets and explanations that might be important for users to understand and follow instructions.
  • The lack of specific issues or problems being addressed, making it difficult to evaluate the effectiveness of the changes.
  • The addition of code or functions without sufficient context or explanation.

The most important findings include:

  • Updates to labeling and descriptions to ensure clarity and accuracy.
  • Changes to code snippets and instructions for improved usability.
  • Removal of irrelevant or unnecessary content.
  • Addition of new documentation files and content for new features or solutions.

These findings suggest that the pull request includes both necessary updates and potentially problematic changes. The reviewer should carefully evaluate the rationale behind the removal of content, the impact on user understanding, and the correctness of added code or functions. Additionally, it may be helpful to provide more detailed explanations or context in the commit messages for a clearer understanding of the changes.

Details

Commit c18e9ccc151897cea54b8f92169252f10956b63b

Key changes in this patch:

  • Updated the label for the category from "Neural Networks for WASI" to "AI inference".
  • Updated the description for the link in the category.

Potential problems:

  • There don't seem to be any potential problems in this patch.

Overall, the changes made in this patch seem to be straightforward updates to the labeling and description of the category.

Commit c4987c813da17ca2956886ce2759acc4ab8526f0

Key Changes:

  • Updated the pytorch.md file in the docs/develop/rust/wasinn directory.
  • Removed 96 lines of content and added 23 lines of new content.
  • Updated the Quick Start section to provide clearer instructions for cloning the WasmEdge-WASINN-examples repo.
  • Removed the instructions for checking libtorch installation and setting the LD_LIBRARY_PATH.
  • Removed the code snippet related to building the wasm file from the Rust source code.
  • Updated the AOT compilation command from wasmedgec to wasmedge compile.
  • Removed the Understand the code section and its associated code snippet.

Potential Problems:

  • The patch removed significant content without providing a reason for the removal or any explanation for the changes being made. The rationale for these changes should be explained.
  • The removed content includes the code snippets and explanations for building and running the example from the Rust source code. The removal of this content may make it difficult for users to understand and follow the instructions.
  • The patch does not mention any specific issues or problems being addressed, which makes it difficult to assess the effectiveness of the changes.

Commit 637999dce0f7c2ad52fea61c77debfbc04a797de

Key changes:

  • The patch updates the pytorch.md file in the docs/develop/rust/wasinn/ directory.
  • It adds instructions to run the inference application in WasmEdge.
  • It updates the directory path in the second code snippet to clone the WasmEdge-WASINN-examples repository.

Potential problems:

  • The changes appear to be straightforward and do not introduce any immediate problems.
  • However, it would be helpful to include more details about the changes made in the commit message.

Commit 7002ff6f9eb42f29008623822acc00a79a31d99a

Key changes:

  • Updated the title of the page from "TensorFlow-Lite Backend" to "TensorFlow Lite Backend".
  • Updated the link to the example project.
  • Updated the prerequisite section to refer to the "WASI-NN plugin" instead of the "Wasi-NN plugin".
  • Updated the example commands to clone the repository to the correct path.
  • Updated the example commands to specify the correct wasm file name.
  • Removed the code for building the Rust project as it is not necessary for the example.
  • Removed the code for converting the input image to tensor data and replaced it with a call to the image_to_tensor() helper function.
  • Updated the code for loading the model, initializing the execution context, and setting the input tensor.
  • Removed the unsafe code for initializing the context and setting the input tensor.
  • Added code for executing the inference using the context and retrieving the output.
  • Updated the code for sorting the output and printing the top-5 classification results.

Potential problems:

  • The patch removes code for building the Rust project from source. This might be an important step for some users who want to build the project themselves.
  • It is unclear why the code for converting the input image to tensor data was removed and replaced with a call to the image_to_tensor() function. The reviewer might need to check if the new function is implemented correctly and if it handles all possible error cases.
  • The patch adds code for loading the model and initializing the execution context, but it is not clear if these steps are necessary for the example to work correctly. The reviewer might need to verify if the changes are correct and if they improve performance as claimed in the patch.
  • The image_to_tensor() function is not defined in the patch. The reviewer might need to check if it is implemented correctly and if it is imported correctly in the code.
  • The patch removes the unsafe code for initializing the context and setting the input tensor. The reviewer might need to check if the unsafe code was removed in favor of safer alternatives and if the changes are correct.
  • The patch updates the code for retrieving the output from the execution context, but it is not clear if the changes are correct. The reviewer might need to verify if the new code correctly retrieves the output and handles any potential errors.
  • The patch updates the code for sorting the output and printing the top-5 classification results. The reviewer might need to check if the changes are correct and if they produce the expected output.

Commit ccb530c847f99b9a5eea616008fc3f065ae36a26

Summary of key changes:

  • The patch updates the file openvino.md in the docs/develop/rust/wasinn directory.
  • The text now references the use of the "Wasi-NN plugin with OpenVINO" instead of "TensorFlow Lite".
  • The patch updates the commands for downloading and running the example.
  • The code examples have been modified to reflect changes in the Wasi-NN library and related APIs.

Potential problems:

  • The image_to_tensor function mentioned in the code examples is no longer defined in the patch. It seems that this function should still be present in the file, or its usage should be removed.
  • The patch removes large portions of the original file, which may cause confusion if there were significant changes to the removed content.

Overall, the key changes in this patch involve updating references to OpenVINO and modifying the commands and code examples accordingly. The potential problems are minor and can be addressed easily.

Commit 13da0e0fe9b90b021b95f2906201843390cca3ef

Key changes:

  • The code in pytorch.md has been updated to fix a documentation issue.

Potential problems:

  • No potential problems were identified in this patch. The changes are minimal and appear to be correct.

Commit e9316361acbbdf332b4ec663b2f28add39d963e7

Key changes:

  • Update the link to main.rs in the documentation for tensorflow_lite.md.
  • Change the description of the code to mention reading the file names from the command line rather than just mentioning reading the files.

Potential problems:

  • No potential problems were identified in this patch. It seems to be a straightforward update to the documentation, fixing a broken link and improving the code description.

Commit c9950a09426348a8c7b47670656a415a9d6ccb8a

Key changes:

  • Added a note asking users to try their own PyTorch models in WasmEdge and provide feedback.

Potential problems:

  • No potential problems identified with this patch. It is a simple addition of a note to encourage user feedback.

Commit a2b24906a8c2ca22ef87961165d28d6477712c02

Key changes:

  • One character has been removed from the text.

Potential problems:

  • There doesn't seem to be any potential problems with this patch. It is a small and straightforward change.

Commit 37c46743520a3490cdeb7baf39fc370485d90b58

Key changes:

  • The chapter title "AI Inference" is renamed to "AI Inference in Rust and WasmEdge"

Potential problems:

  • There don't seem to be any potential problems with this patch. It simply updates the name of a chapter and fixes a broken hyperlink.

Commit 522e6f82deba9c8bbc1f204ca5fd05a7ca6cd053

Key Changes:

  • Updated the sidebar positions in multiple documentation files.
  • Fixed the numbering in the documentation files.
  • Removed the tensorflow.md file and its contents.

Potential Problems:

  • It seems like the changes made in this patch are mostly related to documentation files and their positions in the sidebar. There may not be any functional issues introduced by these changes.
  • However, it is always good to review the updated positions and numbering to ensure they are correct and consistent with the overall structure of the documentation.

Commit af95a8049cbee0b72f87412d360cb83afe51682d

Key changes:

  • Fixed broken links in the docs/embed/go/ai.md file.
  • Removed a section with broken links in the docs/start/install.md file.

Potential problems:

  • No potential problems found. The changes seem to be straightforward fixes to broken links and removing unnecessary content.
  • It may be worth confirming with the author whether the removal of the section in docs/start/install.md was intentional or a mistake.

Commit d40623ae979d820a71631a68c8bcae332e16ce66

Key changes:

  • Fixed broken links in the documentation.
  • Updated the URL path for AI inference in Rust and WasmEdge.
  • Removed some unnecessary content.

Potential problems:

  • None of the changes appear to introduce any problems.

Overall, this patch fixes broken links in the documentation and updates some URLs. It also removes some unnecessary content. There don't appear to be any problems with these changes.

Commit 69193fa831476f0a55461eac26328a6bc2c1023c

Key Changes:

  • The patch removes a broken link in the dapr.md file of the Rust documentation.
  • The broken link is removed from the sentence explaining the functionality of the classify microservice.

Potential Problems:

  • There doesn't appear to be any major problems with this patch. It seems to successfully remove the broken link and does not introduce any new issues.

Commit 216a7c655571853cb438e70c59560ef3d900de86

Key Changes:

  • Added a new documentation file for Mediapipe solutions.
  • Added content to the new Mediapipe documentation file, including a quick start guide, code examples, and available models.
  • Updated the sidebar positions for OpenVINO, PyTorch, and TensorFlow Lite documentation files.

Potential Problems:

  • There don't appear to be any potential problems with these changes.
  • The addition of the new documentation file and its content seems to be in line with the purpose of the repository.

Overall, the changes are focused on adding documentation for Mediapipe solutions and updating the sidebar positions for related documentation files.

Commit 1a914d2e4d91f3dd55f17460c630290a75b37230

Key changes:

  • Added a new file mediapipe.md in the i18n/zh/docusaurus-plugin-content-docs/current/develop/rust/wasinn directory.
  • Added content to the mediapipe.md file, providing instructions and explanations for using the mediapipe-rs crate for data processing using the Mediapipe suite of models.

Potential problems:

  • No potential problems were identified in this patch.

Overall, this patch adds new documentation content for using the mediapipe-rs crate in Rust for data processing using the Mediapipe suite of models. The changes include instructions, code examples, and explanations of various Mediapipe models.

Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
Signed-off-by: Michael Yuan <michael@secondstate.io>
@juntao juntao marked this pull request as ready for review August 20, 2023 22:03
@juntao juntao requested a review from alabulei1 August 20, 2023 22:03
docs/develop/rust/dapr.md Show resolved Hide resolved
@alabulei1 alabulei1 merged commit 60c3841 into main Aug 21, 2023
5 of 6 checks passed
@alabulei1 alabulei1 deleted the juntao-nn branch August 21, 2023 03:13
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