Set metadata when building graph

[dm4]

• Set metadata when building graph
• Set options only if it is specified

This PR depends on:

• https://github.com/second-state/wasmedge-wasi-nn/pull/2
• https://github.com/WasmEdge/WasmEdge/pull/3031

[juntao]

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

---

Overall Summary:

The pull request titled "Set metadata when building graph" includes several key changes to the codebase.

In terms of potential problems and errors, the following issues were identified:

• In the code that checks if the environment variables for options are specified, there is no handling or logging of failures when parsing the environment variables. This could be problematic and should be addressed.
• There are some typos and potential errors, such as the misspelled n-preidct option, which should be corrected to n-predict. Additionally, the reverse_prompt environment variable is not being handled, which might be unintentional.
• The code assumes that all options should be parsed as JSON values, but it might be beneficial to consider different handling for different types of options.
• The code could benefit from better error handling, such as returning appropriate error codes or messages when encountering errors during option parsing or setting.
• The removal of code that sets the options as an input to the context should be further investigated to ensure it doesn't have unintended consequences.
• The patch lacks clear explanations about the purpose and expected behavior of setting metadata when building the graph. Additional information and examples would be helpful for other developers reviewing and using the code.
• The commit message should provide more detailed context and reasoning for the changes.

On the positive side, the patch also includes some important improvements:

• The code now ignores files with the .gguf extension in any directory.
• The options are now set using a options JSON object instead of separate variables.
• The stream_stdout option is now checked using the as_bool() method instead of string parsing.
• The output is printed only if the stream-stdout option is not set to true.
• The README has been updated to provide more detailed information about the parameters and their usage.
• The README now includes a section about setting metadata for inference using the graph builder and input tensor.
• A comment in the main.rs file provides an example of setting options via an input tensor.

In conclusion, the pull request contains both potential issues and important improvements. The identified issues should be addressed, and further improvements can be made to provide more clarity, completeness, and error handling in the code.

Details

Commit b5fa3024e1dcb6f1ef96512b5d65a0452f3f8a42

Key changes:

• Added the pattern "*/.gguf" to ignore all files with the ".gguf" extension in any directory.

Potential problems:

• None observed.

Commit 3dbd8270351f75136ec912d919c794d6d6a8559b

Key changes:

• The code now checks if the environment variables for options are specified before setting the corresponding values in the options JSON object.
• The options JSON object is now used to set the options instead of using separate variables.
• The stream_stdout option is now checked using the as_bool() method instead of parsing a string.
• The code now prints the output only if stream-stdout is not true.

Potential problems:

• In the match statements, it is not clear what should be done if parsing the environment variable fails. Currently, it silently does nothing. It might be better to log an error or handle the failure in some way.
• The n-preidct key in options seems to be a typo and should be n-predict.
• The reverse_prompt environment variable is not being handled, which might be unintentional.
• There is a typo in the line let options["n-preidct"] = serde_json::from_str(val.as_str()).unwrap(), which should be options["n-predict"].
• The code assumes that all options should be parsed as JSON values, but some options might be better represented as other data types. It might be worth considering different handling for different types of options.
• It might be beneficial to validate that the parsed values are of the expected type before setting them in the options object.
• The code could benefit from better error handling, such as returning appropriate error codes or messages when encountering errors during option parsing or setting.

Overall, these findings should be addressed and the code could benefit from further review and improvements.

Commit f6089c22e97be72fc5bfa2972f9657abdde4eb32

Key changes:

• Added .config(options.to_string()) to the GraphBuilder chain.
• Removed code that sets the options as an input to the context.

Potential problems:

1. It seems that the code to set the options as an input to the context has been removed without any clear indication of the reason. This might have unintended consequences and should be further investigated.
2. The patch does not provide any information about the purpose or expected behavior of setting the metadata when building the graph. It would be helpful to include this information to ensure a thorough review.
3. The commit message does not provide sufficient context or reasoning for making the changes. It is recommended to provide more detailed information to help other developers understand the motivation behind the changes.

Commit 2df1eb9715c31d9e82981b2bfc65600e576e5aad

Key changes in the patch:

1. Updated the README.md file to provide more detailed information about the parameters and their usage.
2. Added a section in the README.md file about setting metadata for the inference using the graph builder and the input tensor.
3. Added a comment in the main.rs file to provide an example of setting options via an input tensor.

Potential problems:

1. It would be helpful to provide more examples and explanations for setting metadata using the graph builder and input tensor. This would make it easier for developers to understand and use this feature.
2. The comment in the main.rs file seems to be incomplete. It mentions checking the README for more details, but the necessary instructions and examples are not provided there. It would be better to include the complete example in the comment itself.

Overall, the patch improves the documentation and adds support for setting metadata during graph building and input tensor creation. However, further improvements can be made to provide more clarity and completeness in the documentation and comments.

[hydai]

Hi @dm4 I think we should use a dev branch for the unreleased features. Once the release assets are updated, then we merge it back to master.

[dm4]

Hi @dm4 I think we should use a dev branch for the unreleased features. Once the release assets are updated, then we merge it back to master.

Okay, I have just changed the target branch to the dev branch.

[dm4]

@hydai, according to our previous discussion, I have made updates to the README and have kept the previous metadata setting code as a comment.