Skip to content

wasi_nn_tensorflowlite.cpp: fix get_output return size #4390

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

Merged
merged 1 commit into from
Jun 24, 2025

Conversation

yamt
Copy link
Collaborator

@yamt yamt commented Jun 19, 2025

it should be byte size, not the number of (fp32) values.

i'm ambivalent about how to deal with the compatibility for the legacy wamr-specific "wasi_nn". for now, i avoided changing it. (so that existing tests using the legacy abi, namely test_tensorflow.c and test_tensorflow_quantized.c, pass as they are.) if we have any users who still want to use the legacy abi, i suppose they consider the compatibility is more important than the consistency with other backends.

cf. #4376

@yamt
Copy link
Collaborator Author

yamt commented Jun 19, 2025

Copy link
Collaborator

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with the LLM model, it seems there are assumptions about the format under different tensor->quantization.type. For me, it would be better to add comments to make it easier for readers to follow.

@yamt
Copy link
Collaborator Author

yamt commented Jun 20, 2025

Not familiar with the LLM model, it seems there are assumptions about the format under different tensor->quantization.type.

i'm not quite happy with these assumptions because it makes us incompatible with wasmedge.

For me, it would be better to add comments to make it easier for readers to follow.

totally agree.

it should be byte size, not the number of (fp32) values.

i'm ambivalent about how to deal with the compatibility for
the legacy wamr-specific "wasi_nn". for now, i avoided changing it.
(so that existing tests using the legacy abi, namely test_tensorflow.c
and test_tensorflow_quantized.c, passes as they are.)
if we have any users who still want to use the legacy abi,
i suppose they consider the compatibility is more important
than the consistency with other backends.

cf. bytecodealliance#4376
@lum1n0us lum1n0us merged commit 8289452 into bytecodealliance:main Jun 24, 2025
426 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants