[wasi-nn] Got unexpected results when running tflite_birds_v1_image example about wasm-micro-runtime HOT 10 OPEN

It could be reproduced with commit before #3530, like 8f86b37(WAMR-2.1.0). And WASM_ENABLE_WASI_EPHEMERAL_NN is enabled (so, all imports, wasi_ephemeral_nn.xx, can be linked correctly).

from wasm-micro-runtime.

Let me ask @sunfishcode and @abrown!

from wasm-micro-runtime.

Hi, which commit of WAMR are you using?
Can it be because of #3530 (comment) or are you using WASM_ENABLE_WASI_EPHEMERAL_NN=0?
Thanks!

from wasm-micro-runtime.

@tonibofarull I tried to debug it and basically found the root cause, and submitted PR #3597 for a review. After applying the change, the running result of iwasm is basically same as that of wasmedge, except the order of item 2 and item 3:

wasmedge:
   1.) [166](198)Aix galericulata
   2.) [158](2)Coccothraustes coccothraustes
   3.) [34](1)Gallus gallus domesticus
   4.) [778](1)Sitta europaea
   5.) [819](1)Anas platyrhynchos

iwasm:
   1.) [166](198)Aix galericulata
   2.) [158](2)Coccothraustes coccothraustes
   3.) [34](1)Gallus gallus domesticus
   4.) [778](1)Sitta europaea
   5.) [819](1)Anas platyrhynchos

I am not sure whether the modification is valid/complete, so I changed it to draft, could you help have a look? Thanks.

from wasm-micro-runtime.

BTW, I used API TfLiteTensorCopyFromBuffer and TfLiteTensorCopyToBuffer instead, the related document can be found:

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/core/c/c_api.h#L57-L71
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/core/c/c_api.h#L632-L641

And they are also used in WasmEdge's implementation:
https://github.com/WasmEdge/WasmEdge/blob/master/plugins/wasi_nn/tfl.cpp#L135-L140
https://github.com/WasmEdge/WasmEdge/blob/master/plugins/wasi_nn/tfl.cpp#L166

from wasm-micro-runtime.

This is similar as what it was reported here #2611 (comment).
The conclusion we reached is that if the model is quantized, then the de-quantization must be done within the engine, not by the user.

In this case, from the logs,

[wasi_nn_tensorflowlite.cpp:343 DEBUG] input tensor: (scale, offset) = (0.007812, 128.000000)
[wasi_nn_tensorflowlite.cpp:424 DEBUG] output tensor: (scale, offset) = (0.003906, 0.000000)

Output tensor is dequantized: deq(X) = (X - 0) * 0.003906.
And input tensor is quantized as follows: quant(X) = X / 0.007812 + 128.
Which seems to me that the expected input tensor range is [-1, 1) since quant(-1) ~= 0 and quant(1) ~= 255.

@lum1n0us, @wenyongh, can you try running the inference without #3597 modifications on an image with the expected range of network values? I.e. instead of the normal 0-255 use [-1, 1).

@abrown, do you have any opinion how should we handle this cases?

from wasm-micro-runtime.

@tonibofarull Thanks for the explanation. To be frankly, I am not familiar with wasi-nn, I just tried to debug the issue and happened to make the example run good after comparing the code with wasmedge. From #2611 (comment), I think we don't need to add interface to wasi-nn spec, right? To make it more friendly and flexible, like what you said, maybe we can add options for developer to decide how to use, for example, export API:

wasi_nn_ctx_t wasm_runtime_get_wasi_nn_ctx(wasm_module_inst_t instance);

and add API:

bool wasm_nn_ctx_set_quantize(wasi_nn_ctx_t wasi_nn_ctx, ...);

And in iwasm command line, add argument like iwasm --wasi-nn-quantize=xxx?
This is just an idea, we can change them with better API and argument if needed. And if it is necessary to add interface to wasi-nn spec, I think we had better discuss it first, and then consult with wasi-nn interface designers. How do you think?

from wasm-micro-runtime.

I just tried to debug the issue and happened to make the example run good after comparing the code with wasmedge

Totally! Thanks for the effort.

maybe we can add options for developer to decide how to use

This would be good for the developer using WAMR, but we need to agree on how to do it for any interpreter. So I think it would be good to talk to the WASI-NN API designers.

from wasm-micro-runtime.

Is there any idea from you about how to add the wasi-nn interface? And when should it be called?

from wasm-micro-runtime.

@tonibofarull I forwarded the issue to @abrown in internal meeting and he mentioned there are similar requirements from other developers and they are planning to add wasi-nn interfaces to set the metadata, e.g. to the context and to the tensor. Please feel free to contact him, or maybe we can open an issue in zulip wasi-nn channel for a track:
https://bytecodealliance.zulipchat.com/#narrow/stream/266558-wasi-nn

from wasm-micro-runtime.