Stream: git-wasmtime

Topic: wasmtime / PR #7807 WinML backend for wasi-nn

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 06:50):

jianjunz requested wasmtime-core-reviewers for a review on PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 06:50):

jianjunz requested pchickey for a review on PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 06:50):

jianjunz requested wasmtime-default-reviewers for a review on PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 06:50):

jianjunz opened PR #7807 from jianjunz:winml to bytecodealliance:main:

This change adds Windows Machine Learning backend for wasi-nn. Since WinML is a built-in feature of Windows, this change makes it possible to run wasi-nn on Windows without a third-party machine learning framework.

WinML backend is only available on Windows.

WinML: https://learn.microsoft.com/en-us/windows/ai/windows-ml/

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 07:37):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 07:40):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 08:43):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 24 2024 at 08:43):

jianjunz created PR review comment:

It looks like cargo vet doesn't like this git link. We'll need to release a new version for image2tensor and update the version number here.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 25 2024 at 01:54):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 27 2024 at 00:27):

abrown updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 29 2024 at 02:32):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 29 2024 at 02:35):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 29 2024 at 02:45):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:20):

abrown commented on PR #7807:

@jianjunz:now that #7846 is merged you should be able to rebase on that and get cargo vet to pass.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

Probably an editor auto-format thing: following this project's convention, we can leave this trailing new line in.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

This won't work when both features are enabled. Features are additive, so both backends can be present at the same time.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

I believe this is another place where the tensor item type is hard-coded: we need to be able to handle other types.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

Is this resolved? Looking at the binding code below, I believe we need to handle more than just the f32 case — there are other possible models with other tensor item types.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

Are all of these features needed? I would have assumed that only "AI_MachineLearning" is needed, which adds the right dependent features (see here). The reason I'm thinking about this is that the windows crate totals 11MB and I don't want to unnecessarily bloat Wasmtime binaries on Windows.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

This should handle any kind of model, not just mobilenet. Perhaps just look for the .onnx suffix?

view this post on Zulip Wasmtime GitHub notifications bot (Jan 30 2024 at 18:53):

abrown created PR review comment:

You will want to double-check that this test is actually run. Right now in Wasmtime's CI, only a subset of jobs run to preserve runner time. The Windows "test" job does not run by default but if you add the string prtest:full in any commit, then all jobs will run. Then we can check the logs for the Windows job to ensure this test actually ran.

view this post on Zulip Wasmtime GitHub notifications bot (Jan 31 2024 at 07:34):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:20):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:22):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:22):

jianjunz created PR review comment:

This dependency has been removed. Test image was converted to tensor data offline.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:23):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:23):

jianjunz created PR review comment:

Done. Restored the trailing new line.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:24):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:24):

jianjunz created PR review comment:

"Storage_Streams" and "Foundation_Collections" are needed because some WinML APIs need data structures defined by these two features. "Foundation" is removed.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:33):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:33):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:33):

jianjunz created PR review comment:

We run openvino and winml backend in two different test cases, with different inputs, so only one backend is specified here for testing purpose.

backend.list() is able to list all available backends.

https://github.com/bytecodealliance/wasmtime/blob/9732234291ffc9486b95be5309f920775b20d963/crates/wasi-nn/src/backend/mod.rs#L21-L31

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:33):

jianjunz created PR review comment:

4 is the size of TensorFloat, which is the output data type for probability.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:38):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:38):

jianjunz created PR review comment:

Only f32 is supported right now. A check is added for rejecting unsupported tensor types.

I'm planning to add support for other types in a separate PR since more test data is needed for converging these new tensor types. It may need additional changes for image2tensor to generate the input data.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:39):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 08:39):

jianjunz created PR review comment:

Renamed to model.onnx to align with openvino backend.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 09:30):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 01 2024 at 09:30):

jianjunz created PR review comment:

With prtest:full, this test failed on Windows Server. I guess GitHub Windows Server runner doesn't have Desktop Experience enabled.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 08:10):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 02 2024 at 08:10):

jianjunz created PR review comment:

I'm afraid we need to disable this test at this time. GitHub Actions Windows Server image doesn't have Desktop Experience available, so it cannot be installed.

List of installed or available features:
https://github.com/jianjunz/wasmtime/actions/runs/7752997447/job/21143496106#step:9:1

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 04:03):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 05 2024 at 04:51):

jianjunz has marked PR #7807 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2024 at 09:50):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2024 at 12:41):

jianjunz commented on PR #7807:

Thanks for auditing windows-* crates, but cargo vet still fails after #7900. Only 1 unvetted dependency now.

1 unvetted dependencies:
  windows:0.52.0 missing ["safe-to-deploy"]

view this post on Zulip Wasmtime GitHub notifications bot (Feb 14 2024 at 22:01):

abrown updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 27 2024 at 18:23):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 27 2024 at 18:23):

abrown created PR review comment:

What I mean is: if someone runs cargo test --all-features, both the OpenVINO and WinML statements that assign to let mut backend will be present in the code. Some background: this all.rs file is compiled once by cargo test into a single binary that runs multiple tests, so that compilation will be incorrect because the second let mut backend will shadow the first.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 27 2024 at 18:27):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 27 2024 at 18:27):

abrown created PR review comment:

Yeah, that's unfortunate. Why don't we add a function like check_openvino_is_installed but for WinML: check_winml_is_available. There's probably a way to check with the Windows API if ML system calls are possible and that function could return false if they are not.

That extra runtime check would be in addition to the #[cfg_attr(not(feature = "winml"), ignore)]. That can stay there for the case where the winml feature isn't even enabled.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 27 2024 at 18:32):

abrown submitted PR review:

@jianjunz, this looks good to me with a couple of final points:

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 08:40):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 09:02):

jianjunz commented on PR #7807:

The code is rebased. No conflict. However, your commit https://github.com/bytecodealliance/wasmtime/commit/f81c59d0495630fe82e16d352786aa10358aa281 was dropped because I forgot to pull the code first before rebasing. Could you please submit it again to fix the cargo vet failure? Thanks and sorry for the mistake.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 09:02):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 09:02):

jianjunz submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 09:02):

jianjunz created PR review comment:

check_winml_is_available is added. It returns ok for my Windows 11, but I'll need to find an environment to check if it returns an error when WinML is not available.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 09:02):

jianjunz created PR review comment:

Thanks for the background. It's fixed.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 12:21):

jianjunz updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 15:07):

jianjunz edited a comment on PR #7807:

The code is rebased. No conflict. However, cargo vet reported more unvetted dependencies. I'll check if there is any unnecessary change for cargo.lock.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 28 2024 at 18:56):

abrown updated PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 29 2024 at 23:20):

abrown merged PR #7807.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 01 2024 at 00:30):

jianjunz commented on PR #7807:

Thanks for your review and merging this PR.

Last updated: Dec 23 2024 at 12:05 UTC