abrown opened PR #6821 from abrown:wasi-nn-in-wit
to bytecodealliance:main
:
This change refactors the
wasmtime-wasi-nn
crate to allow access from bothpreview1
andpreview2
ABIs. Though thewasi-nn
specification has included a WIT description for some time, here we use some in-tree files until https://github.com/WebAssembly/wasi-nn/pull/38 is landed. Thepreview2
code is not exercised anywhere yet: ideally this would be wired up once component modelresource
s are fully implemented in Wasmtime.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown has marked PR #6821 as ready for review.
abrown requested pchickey for a review on PR #6821.
abrown requested wasmtime-core-reviewers for a review on PR #6821.
abrown requested wasmtime-default-reviewers for a review on PR #6821.
pchickey submitted PR review:
Adding the representation that is a duplicate of the wasmtime-wit-bindgen generated types creates a lot of noise in this implementation. Wasmtime is going to have the
component-model
feature enabled in its use in the cli as soon as https://github.com/bytecodealliance/wasmtime/pull/6836 lands (should be very soon, just some minor conflicts), at which point you can make wasi-nn available to components as well.I'd rather see this PR land with just the wit-bindgen generated types instead of the duplicates, rather than iterate on that later.
abrown updated PR #6821.
pchickey submitted PR review.
pchickey created PR review comment:
Does this impl need to exist? Where does it get called from?
pchickey submitted PR review:
This looks a lot better, thanks!
pchickey submitted PR review:
This looks a lot better, thanks!
pchickey created PR review comment:
you should be able to erase the
<'a>
here
pchickey created PR review comment:
please use
tracing::debug!
instead ofeprintln!
pchickey created PR review comment:
if the goal of this list of lists is just to pass in a list<u8> for xml and a list<u8> for weights, then can we just make it two arguments that are each list<u8>?
pchickey edited PR review comment.
abrown submitted PR review.
abrown created PR review comment:
Yeah, it's used by
witx.rs
but I guess the more-correct thing is aimpl From<gen::types::GraphEncoding> for BackendKind
. I'll switch to that.
abrown updated PR #6821.
abrown updated PR #6821.
abrown updated PR #6821.
abrown submitted PR review.
abrown created PR review comment:
well, other backends will have different numbers of
list<u8>
to be passed in so the spec has to account for that; theBackend
trait mirrors so that other backends can be implemented.
abrown edited PR review comment.
geekbeast submitted PR review.
geekbeast created PR review comment:
I think eventually we should use named parameters here as well, but @abrown is correct that different frameworks take a different number of options and some even take an execution plan for a model.
abrown merged PR #6821.
Last updated: Nov 22 2024 at 17:03 UTC