search

second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 35 forks source link

[CI] llama: add llava v1.6 #105

Closed dm4 closed 2 months ago

dm4 commented 2 months ago
juntao commented 2 months ago

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

Commit 1a25c5309343b77978d1a0b1b19865d2800b41f2

Key Changes:

  1. The patch adds llava v1.6 to the project.
  2. Changes were made to the YAML file for llama workflow.
  3. Modifications were made in the source code files related to llava: main.rs file had 47 insertions and some deletions, and the .wasm binary file size increased.

Potential Problems:

  1. Deletion of Steps in the YAML Workflow:

    • Some steps in the YAML workflow file have been removed. This might cause issues if those steps were critical for the workflow.

  2. Changes to Binary File Size:

    • The .wasm binary file size increased. This might impact the performance or functionality of the llava module.

  3. Environment Variables and Preloads:

    • Setting environment variables and nn-preloads in the run commands could lead to version-specific dependencies or undesired behavior if not configured correctly.

  4. Hardcoded Values:

    • Hardcoded strings like paths in the run commands could cause issues when the project is deployed on different environments.

  5. Incomplete Information:

    • The patch lacks detailed information about the reasons behind the changes made and any testing performed. More context and testing details should be provided.

It is crucial to test these changes thoroughly, ensure compatibility across different environments, and consider the impact of the increased binary size on the overall performance. Additionally, documenting the changes comprehensively would help in future troubleshooting and maintenance.