search

second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 35 forks source link

[CI] the default ggml is b2534, deprecate b2334 #123

Closed hydai closed 1 month ago

juntao commented 1 month ago

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

Commit fdc1fbdc87dc634e2a3f524214091a900f9059b4

Key changes:

  1. Default ggml plugin is changed to "wasi_nn-ggml" from "wasi_nn-ggml-b2334".

Findings:

  1. The patch updates the plugin setting in the llama.yml workflow file to use the default ggml plugin "wasi_nn-ggml" instead of "wasi_nn-ggml-b2334".
  2. There is a potential problem if the plugin "wasi_nn-ggml" does not fully support the features or requirements that were provided by "wasi_nn-ggml-b2334". This change might cause compatibility issues or functionality loss.
  3. The patch does not provide any explanation or justification for deprecating the "wasi_nn-ggml-b2334" plugin, which could lead to confusion for other developers working on the project.
  4. The patch lacks detailed testing information to ensure that the new default ggml plugin "wasi_nn-ggml" works correctly with the existing workflow and system configurations.

Recommendation:

  1. Ensure that the new default ggml plugin "wasi_nn-ggml" is thoroughly tested to validate its compatibility and functionality.
  2. Consider adding a clear explanation or reference for deprecating the "wasi_nn-ggml-b2334" plugin to help other developers understand the reason behind this change.
  3. Collaborate with the team to gather feedback and ensure that the plugin migration does not negatively impact the project's functionality.