Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation error found in tflite test #3820

Merged

Conversation

lum1n0us
Copy link
Collaborator

No description provided.

@rodjjo
Copy link

rodjjo commented Sep 28, 2024

Is your example compiling even with this typo here ?
wasm_nn_error instead of wasi_nn_error

How about the load function here, are we suppose to include #include "wasi_nn.h" in utils.c as it is declared there ?

Did you get problem with the requirements.txt related to numpy version ?
I was only able to install numpy==1.24.4

In order to test your branch i had to change:
image

Then I used these commands (a bit modified from your Readme.md file):

#!/bin/bash
export EXECUTION_TYPE=cpu 

# Build the runtime
docker build -t wasi-nn-${EXECUTION_TYPE} -f core/iwasm/libraries/wasi-nn/test/Dockerfile.${EXECUTION_TYPE} . || exit 1

# Build wasm app
docker build -t wasi-nn-compile -f core/iwasm/libraries/wasi-nn/test/Dockerfile.compile . || exit 1

# changed this in order to let it have access to the include directory above the test directory
docker run \
     -w /wasi-nn/core/iwasm/libraries/wasi-nn/test \
     -v $PWD/:/wasi-nn \
     -v $PWD/core/iwasm/libraries/wasi-nn/test/models:/models \
     wasi-nn-compile \
     --dir=/ \
     --env="TARGET=cpu" || exit 1

# Run wasm app
docker run \
    -v $PWD/core/iwasm/libraries/wasi-nn/test:/assets \
    -v $PWD/core/iwasm/libraries/wasi-nn/test/models:/models \
    wasi-nn-cpu \
    --dir=/ \
    --env="TARGET=cpu" \
    /assets/test_tensorflow.wasm

After doing that I got:
image

So yes to your question here:
It's not necessary --native-lib anymore. Thank you for updating the readme files.

@lum1n0us
Copy link
Collaborator Author

Is your example compiling even with this typo here ?
wasm_nn_error instead of wasi_nn_error

Sure. I will include it in this PR.

@lum1n0us
Copy link
Collaborator Author

Did you get problem with the requirements.txt related to numpy version ?
I was only able to install numpy==1.24.4

Yes. I met the problem when using 1.26.4. I will include it in this PR

@lum1n0us
Copy link
Collaborator Author

How about the load function here, are we suppose to include #include "wasi_nn.h" in utils.c as it is declared there ?

Sure. I will include it in this PR.

@lum1n0us lum1n0us requested a review from wenyongh September 29, 2024 02:44
@lum1n0us lum1n0us force-pushed the fix/wasi_nn_tflite_compilation branch from 2e111cf to 1588248 Compare September 29, 2024 03:35
@lum1n0us
Copy link
Collaborator Author

I've made some changes. Now the result will look like this.

[test_tensorflow.c:130 INFO] ################### Testing sum...
[wasi_nn.c:269 WARNING] load_by_name_with_config() not found
[wasi_nn_tensorflowlite.cpp:276 WARNING] Default encoding is CPU.
INFO: Created TensorFlow Lite XNNPACK delegate for CPU.
[test_tensorflow.c:132 INFO] ################### Testing max...
[wasi_nn_tensorflowlite.cpp:276 WARNING] Default encoding is CPU.
[test_tensorflow.c:45 INFO] Result: max is 24.000000
[test_tensorflow.c:134 INFO] ################### Testing average...
[wasi_nn_tensorflowlite.cpp:276 WARNING] Default encoding is CPU.
[test_tensorflow.c:64 INFO] Result: average is 12.000000
[test_tensorflow.c:136 INFO] ################### Testing multiple dimensions...
[wasi_nn_tensorflowlite.cpp:276 WARNING] Default encoding is CPU.
[test_tensorflow.c:138 INFO] ################### Testing multiple outputs...
[wasi_nn_tensorflowlite.cpp:276 WARNING] Default encoding is CPU.
[test_tensorflow.c:141 INFO] Tests: passed!

@wenyongh wenyongh merged commit 30539bf into bytecodealliance:main Oct 8, 2024
384 checks passed
@lum1n0us lum1n0us deleted the fix/wasi_nn_tflite_compilation branch November 20, 2024 05:58
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.

4 participants