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

Give an example of llama tflite #165

Merged
merged 6 commits into from
Dec 24, 2024
Merged

Conversation

victoryang00
Copy link
Contributor

working with wamr.

Copy link
Member

juntao commented Dec 19, 2024

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


openvino-mobilenet-image/rust/Cargo.toml

Potential issues

  1. Incorrect Package Name for Llama TFLite Example: The package name wasmedge-wasinn-example-mobilenet-image is incorrect for a llama tflite example, as it suggests an example using MobileNet instead of LLaMA.

  2. Missing TensorFlow Lite Dependency: There is no dependency specified for TensorFlow Lite (tflite) in the [dependencies] section, which is necessary for an example involving TFLite models like LLaMA.

  3. Version Incompatibility Concerns: Using a specific version of wasi-nn (0.6.0) without ensuring compatibility with TensorFlow Lite can lead to runtime issues or lack of support for required features in the LLaMA model.

Summary of changes

  • Updated wasi-nn version: Changed from 0.4.0 to 0.6.0 to ensure compatibility with newer features or bug fixes.
    No other significant changes in the provided patch affecting the example of LLaMA TFLite directly, focusing mainly on dependency updates.

openvino-mobilenet-raw/rust/Cargo.toml

Potential issues

  1. Incorrect Package Name: The package name wasmedge-wasinn-example-mobilenet does not reflect the subject of "llama tflite", indicating a mismatch between the project's purpose and its declared identity.
  2. Mismatched Dependencies: Dependency on wasi-nn instead of TensorFlow Lite libraries suggests the codebase is not set up for TFLite models, which contradicts the specified example theme.
  3. Unconfigured Workspace: The [workspace] section is empty, which can lead to issues in a multi-package repository by not properly defining how packages are related and shared.

Summary of changes

    • Updated wasi-nn dependency version from 0.4.0 to 0.6.0.
  • No other significant changes made in the patch.

openvino-road-segmentation-adas/openvino-road-seg-adas/Cargo.toml

Potential issues

  1. The name field in the package metadata is "openvino-road-seg-adas", which does not match the subject of "Give an example of llama tflite"; this suggests a mismatch between the project name and its content or purpose.
  2. The dependency on wasi-nn version "0.6.0" may be outdated or incompatible with the intended use case for LLAMA TFLite models, leading to potential integration issues.
  3. The extensive list of features enabled in the image crate might include unnecessary dependencies, increasing the build size and potentially causing conflicts or slowing down compilation times.

Summary of changes

  • Key Changes:
  • Updated wasi-nn dependency version: Changed from "0.4.0" to "0.6.0", which likely includes bug fixes, performance improvements, or new features relevant to the project.
  • No significant changes in other dependencies: The only notable change is in the wasi-nn dependency, with all other dependencies remaining unchanged.
  • Retained comprehensive image feature support: The image crate's features configuration stayed intact, preserving support for various image formats (gif, jpeg, etc.).

tflite-birds_v1-image/rust/Cargo.toml

Potential issues

  1. Version Pinning: Version pinning for dependencies can lead to issues with updates and security patches; consider using version ranges or a Cargo.toml feature that allows for more flexible dependency management.
  2. Feature Overlap: The default-features = false followed by enabling many features from the image crate might be redundant if all needed features are explicitly listed; ensure this is intentional to avoid unnecessary complexity.
  3. Missing Workspace Configuration: The [workspace] section is present but empty, which could lead to confusion or errors if the intention was to include sub-crates in a workspace; define members or other configurations as needed.

Summary of changes

    • Updated wasi-nn version: Changed the dependency from "0.4.0" to "0.6.0".
  • This upgrade likely includes bug fixes, performance improvements, and possibly new features relevant to running neural networks in WebAssembly environments.
  • Ensures compatibility with the latest developments in the wasi-nn library, which is crucial for integrating llama tflite models effectively.

wasmedge-tf-llama/rust/Cargo.toml

Potential issues

  1. Outdated Dependency: The wasi-nn dependency is specified as version "0.1.0", which might not support the latest features or have critical bug fixes; ensure using the latest version.

  2. Missing License Information: There is no license field specified in the [package] section, which can cause issues with distribution and usage of the package.

  3. Hardcoded Dependencies: The dependency versions are hardcoded (e.g., wasmedge_tensorflow_interface = "0.3.0"), which can lead to build failures if a newer, incompatible version is required in future changes; consider using version ranges or semantic versioning constraints.

Summary of changes

    • Added a new Cargo.toml file for a project named "wasmedge-tf-example-llama".
  • Included dependencies such as wasmedge_tensorflow_interface, wasi-nn, and others essential for TensorFlow Lite operations with WasmEdge.
  • Configured the project to use Rust edition 2021 and specified that it should not be published.

wasmedge-tf-llama/rust/src/main.rs

Potential issues

N/A

Summary of changes

N/A

Copy link
Member

@hydai hydai left a comment

Choose a reason for hiding this comment

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

I got a little confused. The title of this PR said it plans to add a new example for Llama TFLite. However, the README.md and Cargo.toml are all about MobileNet. Could you please check if the README and metadata are correct?

@victoryang00
Copy link
Contributor Author

fixed

@hydai hydai merged commit 56d497a into second-state:master Dec 24, 2024
0 of 3 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