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] ggml: add Qwen2-VL example #166

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

[Example] ggml: add Qwen2-VL example #166

wants to merge 3 commits into from

Conversation

dm4
Copy link
Member

@dm4 dm4 commented Dec 24, 2024

Copy link
Member

juntao commented Dec 24, 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/llama.yml

Potential issues

N/A

Summary of changes

N/A

wasmedge-ggml/qwen2vl/Cargo.toml

Potential issues

  1. Version Pinning: The dependencies (serde_json and wasmedge-wasi-nn) are pinned to specific versions, which can lead to potential issues with newer updates not being tested or integrated. Consider using version ranges instead.
  2. Missing License Information: There is no license specified in the package metadata, which could lead to ambiguity about how the software can be used and distributed.
  3. Potential Edition Mismatch: The edition = "2021" may not be compatible with all dependencies or the target environment, ensure that this edition is supported by all listed dependencies.

Summary of changes

  • Added a new Cargo.toml file: Initializes a Rust package named "wasmedge-ggml-qwen2vl".
  • Set the Rust edition to 2021.
  • Included dependencies: Added serde_json version 1.0 and wasmedge-wasi-nn version 0.7.1 for JSON processing and neural network functionalities, respectively.

wasmedge-ggml/qwen2vl/src/main.rs

Potential issues

  1. Error Handling: The get_options_from_env function uses unwrap() when parsing JSON strings from environment variables, which can cause a panic if the string is not valid JSON. Use Result::map_err() or similar to handle errors gracefully.

  2. Buffer Overflow Risk: In get_data_from_context, the code assumes that the output size will never exceed MAX_OUTPUT_BUFFER_SIZE. However, if the actual output exceeds this, it can lead to a buffer overflow. Ensure the buffer size is dynamically adjusted based on the actual output size returned by context.get_output().

  3. Redundant Metadata Retrieval: The metadata is retrieved twice in non-interactive mode (get_metadata_from_context(&context) is called twice). This redundant call should be removed for efficiency and to avoid potential inconsistencies if the metadata changes between calls.

Summary of changes

  • Key Changes Summary:
  • Added Qwen2-VL Example: Introduced a complete Rust program to interact with the Qwen2-VL model using WasmEdge-WASINN, handling image and text inputs.
  • Environment Variable Parsing: Implemented a function get_options_from_env to read required (mmproj, image) and optional parameters (enable_log, ctx_size, n_gpu_layers) from environment variables.
  • Interactive Mode Support: Enabled both interactive and non-interactive modes for the application, with non-interactive mode handling a prompt provided as a command-line argument.

wasmedge-ggml/qwen2vl/wasmedge-ggml-qwen2vl.wasm

Potential issues

N/A

Summary of changes

N/A

@dm4 dm4 marked this pull request as draft December 25, 2024 08:03
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