wasm-micro-runtime icon indicating copy to clipboard operation
wasm-micro-runtime copied to clipboard

[wasi-nn] Got unexpected results when running tflite_birds_v1_image example

Open lum1n0us opened this issue 1 year ago • 10 comments

If compared with the expected result from wasmedge, got a different result when running a tflite_birds_v1_image with iwasm.

The expected result:

 '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'

WAMR result:

 '1.) [526](136)Cathartes burrovianus',
 '2.) [482](128)Turdus plumbeus',
 '3.) [590](128)Anous stolidus',
 '4.) [614](128)Buteogallus anthracinus',
 '5.) [826](128)Anas clypeata'

@tonibofarull Would mind taking a look at it? Even debugging suggestions are helpful.


Log:

$ pwd
somewhere/WasmEdge-WASINN-examples/tflite-birds_v1-image

$ somewhere/wasm-micro-runtime/product-mini/platforms/linux/build/iwasm --native-lib=somewhere/wasm-micro-runtime/product-mini/platforms/linux/build/libwasi-nn-tflite.so --map-dir=.:. ./wasmedge-wasinn-example-tflite-bird-image.wasm ./lite-model_aiy_vision_classifier_birds_V1_3.tflite bird.jpg

Read graph weights, size in bytes: 3561598
[wasi_nn.c:194 DEBUG] Running wasi_nn_load [encoding=4, target=0]...
[wasi_nn_app_native.c:54 DEBUG] Graph builder array contains 1 elements
[wasi_nn_app_native.c:90 DEBUG] Graph builder 0 contains 3561598 elements
[wasi_nn.c:82 DEBUG] Initializing wasi-nn context
[wasi_nn_tensorflowlite.cpp:446 DEBUG] Initializing models.
[wasi_nn_tensorflowlite.cpp:451 DEBUG] Initializing interpreters.
[wasi_nn.c:136 DEBUG] Returning ctx
[wasi_nn.c:225 DEBUG] wasi_nn_load finished with status 0 [graph=0]
[wasi_nn.c:277 DEBUG] Running wasi_nn_init_execution_context [graph=0]...
[wasi_nn.c:136 DEBUG] Returning ctx
INFO: Initialized TensorFlow Lite runtime.
[wasi_nn_tensorflowlite.cpp:278 WARNING] Default encoding is CPU.
[wasi_nn.c:298 DEBUG] wasi_nn_init_execution_context finished with status 0 [ctx=0]
Loaded graph into wasi-nn with ID: Graph#0
Read input tensor, size in bytes: 150528
[wasi_nn.c:308 DEBUG] Running wasi_nn_set_input [ctx=0, index=0]...
[wasi_nn.c:136 DEBUG] Returning ctx
[wasi_nn_app_native.c:166 DEBUG] Converting tensor_wasm to tensor
[wasi_nn_app_native.c:158 DEBUG] Number of dimensions: 4
[wasi_nn_app_native.c:186 DEBUG] Dimension 0: 1
[wasi_nn_app_native.c:186 DEBUG] Dimension 1: 224
[wasi_nn_app_native.c:186 DEBUG] Dimension 2: 224
[wasi_nn_app_native.c:186 DEBUG] Dimension 3: 3
[wasi_nn_app_native.c:188 DEBUG] Tensor type: 2
[wasi_nn_app_native.c:189 DEBUG] Total number of elements: 150528
[wasi_nn_tensorflowlite.cpp:296 DEBUG] Number of tensors (1)
[wasi_nn_tensorflowlite.cpp:343 DEBUG] input tensor: (scale, offset) = (0.007812, 128.000000)
[wasi_nn.c:334 DEBUG] wasi_nn_set_input finished with status 0
[wasi_nn.c:341 DEBUG] Running wasi_nn_compute [ctx=0]...
[wasi_nn.c:136 DEBUG] Returning ctx
[wasi_nn.c:355 DEBUG] wasi_nn_compute finished with status 0
[wasi_nn.c:371 DEBUG] Running wasi_nn_get_output [ctx=0, index=0]...
[wasi_nn.c:136 DEBUG] Returning ctx
[wasi_nn_tensorflowlite.cpp:381 DEBUG] Number of tensors (1)
[wasi_nn_tensorflowlite.cpp:424 DEBUG] output tensor: (scale, offset) = (0.003906, 0.000000)
[wasi_nn.c:399 DEBUG] wasi_nn_get_output finished with status 0 [data_size=965]
   1.) [526](136)Cathartes burrovianus
   2.) [482](128)Turdus plumbeus
   3.) [590](128)Anous stolidus
   4.) [614](128)Buteogallus anthracinus
   5.) [826](128)Anas clypeata
[wasi_nn.c:463 DEBUG] --|> deinit_native_lib
[wasi_nn.c:148 DEBUG] Freeing wasi-nn
[wasi_nn.c:149 DEBUG] -> is_model_loaded: 1
[wasi_nn.c:150 DEBUG] -> current_encoding: 4
[wasi_nn_tensorflowlite.cpp:475 DEBUG] Freeing memory.
[wasi_nn_tensorflowlite.cpp:513 DEBUG] Memory free'd.

lum1n0us avatar Jun 20 '24 12:06 lum1n0us

Hi, which commit of WAMR are you using? Can it be because of https://github.com/bytecodealliance/wasm-micro-runtime/pull/3530#discussion_r1639662614 or are you using WASM_ENABLE_WASI_EPHEMERAL_NN=0? Thanks!

tonibofarull avatar Jun 20 '24 16:06 tonibofarull

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

lum1n0us avatar Jun 20 '24 23:06 lum1n0us

@tonibofarull I tried to debug it and basically found the root cause, and submitted PR https://github.com/bytecodealliance/wasm-micro-runtime/pull/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.

wenyongh avatar Jul 04 '24 07:07 wenyongh

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

wenyongh avatar Jul 04 '24 12:07 wenyongh

This is similar as what it was reported here https://github.com/bytecodealliance/wasm-micro-runtime/issues/2611#issuecomment-1741595890. 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 https://github.com/bytecodealliance/wasm-micro-runtime/pull/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?

tonibofarull avatar Jul 04 '24 12:07 tonibofarull

@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 https://github.com/bytecodealliance/wasm-micro-runtime/issues/2611#issuecomment-1743801757, 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?

wenyongh avatar Jul 05 '24 01:07 wenyongh

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.

tonibofarull avatar Jul 05 '24 07:07 tonibofarull

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

wenyongh avatar Jul 05 '24 07:07 wenyongh

Let me ask @sunfishcode and @abrown!

tonibofarull avatar Jul 05 '24 07:07 tonibofarull

@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

wenyongh avatar Jul 10 '24 01:07 wenyongh