Stream: git-wasmtime

Topic: wasmtime / PR #6821 wasi-nn: refactor to allow `preview2`...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 21:29):

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 both preview1 and preview2 ABIs. Though the wasi-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. The preview2 code is not exercised anywhere yet: ideally this would be wired up once component model resources are fully implemented in Wasmtime.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 21:33):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 21:35):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 21:40):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:10):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 08 2023 at 22:11):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 15:58):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 22:11):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 22:12):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 22:27):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 23:19):

abrown has marked PR #6821 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 23:19):

abrown requested pchickey for a review on PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 23:19):

abrown requested wasmtime-core-reviewers for a review on PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 09 2023 at 23:19):

abrown requested wasmtime-default-reviewers for a review on PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 15 2023 at 23:11):

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.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 16:55):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:30):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:30):

pchickey created PR review comment:

Does this impl need to exist? Where does it get called from?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

pchickey submitted PR review:

This looks a lot better, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

pchickey submitted PR review:

This looks a lot better, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

pchickey created PR review comment:

you should be able to erase the <'a> here

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

pchickey created PR review comment:

please use tracing::debug! instead of eprintln!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:33):

pchickey edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:36):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:36):

abrown created PR review comment:

Yeah, it's used by witx.rs but I guess the more-correct thing is a impl From<gen::types::GraphEncoding> for BackendKind. I'll switch to that.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:38):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 17:57):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:01):

abrown updated PR #6821.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:04):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:04):

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; the Backend trait mirrors so that other backends can be implemented.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:04):

abrown edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:47):

geekbeast submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 18:48):

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.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 16 2023 at 20:09):

abrown merged PR #6821.


Last updated: Oct 23 2024 at 20:03 UTC