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

Add neural speed backend #226

Merged
merged 3 commits into from
Jun 14, 2024
Merged

Add neural speed backend #226

merged 3 commits into from
Jun 14, 2024

Conversation

grorge123
Copy link
Contributor

Explanation

Add neural speed backend install tutorial.

Related issue

WasmEdge/WasmEdge#3260

What type of PR is this

/kind documentation

Copy link
Collaborator

alabulei1 commented Apr 25, 2024

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


Overall, the Pull Request titled "Add neural speed backend" introduces documentation updates and instructions for integrating the WASI-NN Neural Speed Backend. The patch brings valuable information, but there are potential issues that need attention.

The highlighted potential problems include the introduction of a new dependency on Neural Speed without addressing version compatibility or conflicts with existing dependencies. This could lead to compatibility issues in the future. Additionally, the patch includes sudo usage for package installation, posing security risks if the source is not verified. Clear verification and package manager usage guidance should be provided. Lastly, the configuration complexity may increase with the addition of a new backend, emphasizing the need for streamlined and user-friendly instructions to prevent confusion or errors during the build process.

Furthermore, the update of the backend name from "OpenVINO" to "Neural Speed" raises concerns about inconsistent terminology. Ensuring uniformity in naming conventions throughout the codebase is crucial, and verifying the official designation of "Neural Speed" is recommended.

In summary, while the patch enhances documentation and integration instructions, addressing dependency issues, security concerns, and maintaining consistent terminology are crucial for a successful integration of the Neural Speed backend into the system.

Details

Commit 275151dbe368b98ab2578660862f313961d30495

Key changes:

  • Added documentation on building WasmEdge with the WASI-NN Neural Speed Backend.
  • Provided instructions for installing Neural Speed and building WasmEdge with the new backend.

Potential problems:

  1. Dependency issues: The patch introduces a new dependency on Neural Speed without considering potential conflicts with existing dependencies or versions. There should be a note on version compatibility or any known issues with specific versions.

  2. Security concerns: Installing packages using sudo without verifying the source or contents can pose security risks. It's advisable to include information on verifying the source or using package managers where possible.

  3. Configuration complexity: Adding a new backend increases the complexity of the build process. It's important to provide clear instructions and ensure that the process is streamlined to avoid user confusion or errors.

Overall, the patch introduces valuable information for utilizing the Neural Speed backend but could benefit from addressing the potential problems mentioned above.

Commit f2ce7a9c67c0f6cf672021f5747096e25a99f653

Key Changes:

  1. Updated the backend name in the WASI-NN documentation from "OpenVINO" to "Neural Speed".

Potential Problems:

  1. Inconsistent Terminology: The patch is updating the backend name from "OpenVINO" to "Neural Speed". It's important to ensure consistency in naming conventions throughout the codebase. Confirm if "Neural Speed" is the officially designated name and update any related references accordingly.

Overall, the changes seem straightforward and mainly involve updating the documentation to reflect the correct backend name. It would be beneficial to verify if there are any additional code changes or impacts on other parts of the system due to this modification.

Signed-off-by: grorge <[email protected]>
@hydai
Copy link
Member

hydai commented Jun 13, 2024

@grorge123
Could you please fix the conflict in wasi_nn.md?

@hydai hydai merged commit a2dd6ee into WasmEdge:main Jun 14, 2024
6 checks passed
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.

3 participants