Skip to content

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Jun 5, 2025

tested with:

openvino-mobilenet-raw has the same issue. it can be fixed by transposing the tensor file as suggested in:
as described in
bytecodealliance/wasmtime#10867 (comment)

i guess these tests have the same origin as wasmtime's classification-example and share the same bug.
see
bytecodealliance/wasmtime#10867 for details of the bug.

tested with:

* wasmtime
* wasm-micro-runtime with bytecodealliance/wasm-micro-runtime#4308

openvino-mobilenet-raw has the same issue.  it can be fixed by
transposing the tensor file as suggested in:
as described in
bytecodealliance/wasmtime#10867 (comment)

i guess these tests have the same origin as wasmtime's
classification-example and share the same bug.
see
bytecodealliance/wasmtime#10867
for details of the bug.
Copy link
Member

juntao commented Jun 5, 2025

Hello, I am a code review agent on flows.network. Here are my reviews of changed source code files in this PR.


openvino-mobilenet-image/rust/src/imagenet_classes.rs

Potential issues

  1. Incorrect String Literal: The string "devil\"s darning needle" contains an escaped double quote (") which is unnecessary and incorrect in Rust string literals. Use a raw string or escape the backslash properly: r#"devil's darning needle"# or "devil's darning needle".

  2. Array Length Mismatch: The array IMAGENET_CLASSES is declared with a fixed size of 1000 elements ([&str; 1000]). However, the number of entries provided is less than 1000, which will cause a compilation error. Ensure that all 1000 classes are included or adjust the array's declaration to match the actual number of entries.

  3. Lack of Comments: While not a coding issue per se, the absence of comments or documentation around the IMAGENET_CLASSES array makes it harder for future developers to understand its purpose and usage. Consider adding a comment explaining what this array represents.

Summary of changes

    • Removed newline issue: The patch corrects a missing newline at the end of the file, which is now properly terminated.
  • No structural code changes: No actual tensor shape or logical changes were made; only formatting was adjusted.

openvino-mobilenet-image/rust/src/main.rs

Potential issues

  1. The image::io::Reader::open function might not be available in the environment you're working on.
  2. The DynamicImage::resize_exact method requires the dimensions to be a multiple of 32, which is a requirement for some OpenVINO models. If these dimensions are not multiples of 32, the resize operation will fail.
  3. The image_to_tensor function assumes that the image pixels are in BGR order, but it does not handle the case where the input image might have different color spaces.

Summary of changes

  • Fixed tensor shape for openvino-mobilenet-image.
  • Transposed BGR image from [height, width, 3] to [3, height, width].
  • Added padding if the input dimensions do not match the expected dimensions.

@hydai hydai merged commit eee8c0d into second-state:master Jun 5, 2025
1 check passed
@hydai
Copy link
Member

hydai commented Jun 5, 2025

Thanks

@yamt
Copy link
Contributor Author

yamt commented Jun 5, 2025

@hydai
can you or someone take care of the other example (openvino-mobilenet-raw) and wasmedge?
i have not used wasi-nn with wasmedge.

@hydai
Copy link
Member

hydai commented Jun 5, 2025

Sure, we will track this issue at WasmEdge/WasmEdge#4142.

@yamt
Copy link
Contributor Author

yamt commented Jun 5, 2025

Sure, we will track this issue at WasmEdge/WasmEdge#4142.

thank you.

@yamt
Copy link
Contributor Author

yamt commented Jun 17, 2025

openvino-mobilenet-raw has the same issue.

openvino-road-segmentation-adas too.

@hydai
Copy link
Member

hydai commented Jun 17, 2025

Thanks, @yamt, for the reminder. I forgot to keep track of these two examples.
Created #192 for tracking them.

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