search

WasmEdge / docs

https://wasmedge.org/docs/
Apache License 2.0
16 stars 55 forks source link

Add neural speed backend #226

Closed grorge123 closed 1 month ago

grorge123 commented 2 months ago

Explanation

Add neural speed backend install tutorial.

Related issue

https://github.com/WasmEdge/WasmEdge/issues/3260

What type of PR is this

/kind documentation

alabulei1 commented 2 months ago

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:

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.

hydai commented 1 month ago

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