second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 36 forks source link

[feat] New example `Belle-Llama2-13B-GGUF` #53

Closed apepkuss closed 7 months ago

juntao commented 7 months ago

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


Overall Summary:

This pull request introduces a new example and provides the necessary code and documentation for it. However, there are several issues and findings that should be addressed to ensure compatibility, functionality, and maintainability.

Potential Issues:

Important Findings:

In summary, while the pull request introduces a new example and provides documentation, there are potential issues with dependencies, error handling, code organization, and missing information. These should be addressed to ensure compatibility, functionality, maintainability, and clarity in the project.

Details

Commit 7b99b26cc769e84ec2da5b43281ec6f9f3612896

Key changes in the patch:

  1. Added a new example called belle-llama2-13B in the wasmedge-ggml-belle-chat project.
  2. Added a new file Cargo.toml with project metadata and dependency specifications.
  3. Added a new file main.rs with the implementation of the belle-llama2-13B chat prompt.
  4. Modified the main function to handle the new chat prompt and execute the chat process.
  5. Added new dependencies in the Cargo.toml file.

Potential problems:

  1. The patch adds a new dependency called wasi-nn with a specific branch. This may result in compatibility issues if the branch is not up to date or changes are made to the dependencies.
  2. The patch introduces the use of the chat-prompts, clap, and endpoints libraries. If these libraries are not properly implemented or maintained, it may cause issues in the functionality of the code.
  3. The code lacks proper error handling and error messages. This may make it difficult to identify and debug problems.
  4. The code has some unused variables and unused imports (OnceCell, print_separator function).
  5. There is an infinite loop in the main function. Although it is intentional for the chat process, it could become problematic if there are no break conditions or ways to exit gracefully.
  6. The code does not provide any comments or documentation to explain the purpose and functionality of the different components.

Overall, the patch introduces a new example and the necessary code to implement it. However, there are potential problems with dependencies, error handling, and maintainability that should be addressed.

Commit 0f13dc71459844e22410a5eb48bf32a05f343d46

Key changes in this patch:

  1. Added a new README file for the wasmedge-ggml-belle-chat directory.
  2. The README provides instructions for installing dependencies, preparing the WASM application, getting the model, executing the application, handling errors, and setting parameters.
  3. The README includes code snippets and console outputs for clarity.

Potential problems:

  1. The ownership and permissions for the README file should be checked to ensure they are consistent with the project.
  2. The URLs for the model and the llama.cpp GitHub repository should be validated to ensure they are correct and up-to-date.
  3. The command used to install the libopenblas-dev package on Ubuntu may require sudo permissions. The README should clarify that if the user is not root, they may need to use sudo when running the command.
  4. The README includes a code snippet with environment variable assignments before the wasmedge command. It's possible that some environment variables might conflict or be overridden if they are already set in the user's system. This should be clarified in the README, and users should be advised to review their existing environment variables before running the code.
  5. The README mentions the supported parameters (LLAMA_LOG, LLAMA_N_CTX, LLAMA_N_PREDICT) and how to set them using environment variables. However, it's not clear if these parameters have default values or what impact changing them might have on the application. This should be clarified in the README to provide better guidance to users.
  6. The patch does not provide any information about the changes made to the code or functionality of the project. It focuses solely on providing instructions for setting up and running the application. If there were any code changes, they are not documented in this patch.

Overall, the patch adds a new README file with comprehensive instructions for setting up and running the application. However, further clarification and validation are needed for some steps and potential issues. Additionally, it would be helpful to include information about any code changes made in this pull request.

apepkuss commented 7 months ago

@hydai Please help review this PR. Thanks a lot!

apepkuss commented 7 months ago

@hydai @dm4 The review issues has been fixed. Please review the changes. Thanks a lot!