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

[Examples] Add wasmedge-ggml-llama examples #28

Merged
merged 5 commits into from
Sep 6, 2023
Merged

Conversation

dm4
Copy link
Member

@dm4 dm4 commented Sep 4, 2023

  • SHOULD be merged after WasmEdge 0.13.4 released.
  • Add wasmedge-ggml-llama example with model llama-2-7b-chat.ggmlv3.q4_0.bin.

Copy link
Member

juntao commented Sep 4, 2023

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


Overall Summary:

In this GitHub Pull Request, several changes have been made to add examples for the wasmedge-ggml-llama crate. The changes include adding new files, modifying the .gitignore file, creating a workflow, and updating command parameters.

Potential Issues and Errors:

  • The code in main.rs needs to be reviewed to ensure adherence to best practices and avoid security vulnerabilities or performance issues.
  • The use of sudo in the workflow steps may cause issues on hosted runners where root privileges are not available.
  • The script for installing WasmEdge, WASI-NN, and GGML assumes a fixed version, which may become outdated. More flexible or up-to-date installation methods should be considered.
  • The URL used to download the GGML model file may not be reliable in the long term. Consistent access to the model file should be ensured.
  • It is unclear why the model version has been changed, and compatibility with the existing code should be verified.

Important Findings:

  • The addition of the wasmedge-ggml-llama crate as a dependency allows for the loading and running of models using wasi-nn and wasmedge.
  • The workflow created in llama.yml automates the building and testing of the llama2 examples on an Ubuntu 20.04 runner.
  • The change in the curl command updates the model version being downloaded.

In conclusion, the changes in this Pull Request seem to be straightforward, but further information and testing are needed to ensure the compatibility and functionality of the updated code. It is recommended to address the potential issues and provide more context for changes to improve clarity and maintainability.

Details

Commit b4c6351303c033c492ca153fc933f037cc14591e

Key changes:

  • Added a new directory named wasmedge-ggml-llama containing three new files: Cargo.toml, README.md, and main.rs.
  • Modified the .gitignore file to include the newly added binary file llama-2-7b-chat.ggmlv3.q4_0.bin.
  • Added the wasmedge-ggml-llama crate as a dependency in Cargo.toml.
  • Created a README.md file with instructions for building and executing the example.
  • Created a main.rs file with code for loading and running a model using wasi-nn and wasmedge.

Potential problems:

  • There are no potential problems identified in this patch. However, it's important to review the code in main.rs to ensure it adheres to best practices and does not introduce any security vulnerabilities or performance issues.

Commit d719afe7408c5ea3e7ef5afc2c559f7291fb8b9e

Key changes:

  • Added a new file called llama.yml in the .github/workflows directory.
  • The llama.yml file contains a workflow for building and testing llama2 examples.
  • The workflow is triggered on workflow dispatch, push, and pull request events.
  • The workflow runs on an Ubuntu 20.04 runner.
  • Several steps are defined within the workflow, including installing necessary packages, adding Rust target for wasm, installing WasmEdge + WASI-NN + GGML, and running an example.

Potential problems:

  • The use of sudo in the workflow steps may cause issues when running the workflow on a hosted runner where root privileges are not available.
  • The script used for installing WasmEdge, WASI-NN, and GGML assumes a fixed version 0.13.4. This may become outdated over time, and it might be better to use a more flexible or up-to-date installation method.
  • The script downloads a GGML model file (llama-2-7b-chat.ggmlv3.q4_0.bin) from a specific URL. This URL may not be reliable in the long term, and it would be better to ensure that the model file can be fetched consistently.
  • The workflow assumes that the working directory is wasmedge-ggml-llama without explicitly specifying it. This could cause issues if the workflow is triggered from a different directory.

Commit 25ae1bdcb04d12ad9a0ee32eb1f07545e4bcd71d

Key changes:

  • The URL in the curl command has been updated to download a different version of the llama-2-7b-chat.ggml model.

Potential problems:

  • It is unclear why the model version has been changed. The reason for this change should be documented in the pull request or commit message.
  • It is important to ensure that the new version of the model is compatible with the existing code. Without further information, it is not possible to determine whether any code modifications are needed to support this new model version.

Overall, the change seems to be straightforward, but further information and testing may be needed to ensure the compatibility and functionality of the updated code with the new model version.

Commit 429e1f984dc0b40701892a88b73cb73056125500

Key Changes:

  • The line wasmedge --dir .:. --nn-preload default:GGML:CPU:llama-2-7b-chat.ggmlv3.q4_0.bin wasmedge-ggml-llama.wasm default 'Once upon a time, ' has been updated to wasmedge --dir .:. --nn-preload default:GGML:CPU:llama-2-7b-chat.ggmlv3.q2_K.bin wasmedge-ggml-llama.wasm default 'Once upon a time, '.

Potential Problems:

  • It seems that the change made in this patch is just a correction in the filename mentioned in the command. However, without more context or information, it is not clear whether this change is intended or if there was an error in the previous version. If this change is intended, it would be recommended to provide a brief explanation in the commit message or comments.
  • Aside from the filename change, there doesn't appear to be any other significant changes in this patch. If there were other changes made related to the llama.yml file, they are not included in this patch. It would be helpful to review the entire pull request to ensure all necessary changes are included.

Commit 81046c6486cc2c022e2448a678bcf08934ac21b3

Key changes in the patch:

  • The llama.yml workflow file is being updated.

Potential problems:

  • It seems that the URLs for downloading the model files have been changed. The old URL was https://huggingface.co/localmodels/Llama-2-7B-Chat-ggml/resolve/main/llama-2-7b-chat.ggmlv3.q2_K.bin, and it has been replaced with https://huggingface.co/TheBloke/orca_mini_3B-GGML/resolve/main/orca-mini-3b.ggmlv3.q4_0.bin. The reviewer should confirm if this change is intentional and the new URL is correct.
  • The wasmedge-ggml-llama executable is being run with different parameters. The reviewer should analyze if this change is correct and if it aligns with the intended functionality.

Overall, the patch seems to update the workflow file to reflect changes in the URL for model file download and the parameters passed to the wasmedge-ggml-llama executable. The reviewer should confirm the correctness and intentionality of these changes.

@juntao
Copy link
Member

juntao commented Sep 6, 2023

The llama2 CI passed. I think we can merge this. Thanks. @dm4 @hydai

@hydai hydai marked this pull request as ready for review September 6, 2023 05:55
@hydai hydai merged commit 83bb521 into master Sep 6, 2023
4 of 6 checks passed
@dm4 dm4 deleted the wasmedge-ggml-llama branch November 9, 2023 05:58
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