wasi-nn: add feature to use custom ONNX Runtime #11060
Conversation
crates/wasi-nn/Cargo.toml
Outdated
| onnx = ["dep:ort"] | ||
| onnx = ["ort/download-binaries"] | ||
| # Use locally built ONNX Runtime. | ||
| onnx-local-rt = ["dep:ort"] |
There was a problem hiding this comment.
What if we flipped this around?
onnxwould just havedep:ortandonnx-downloadwould bring in theort/download-binariesfeature
(I'm no Cargo features expert but it does seem to conform better to the idea that they should be additive).
There was a problem hiding this comment.
I am open to both options. But this would be a breaking change. If we flipped this around, existing users who are using onnx feature would have to switch to onnx-download feature. Is this OK?
There was a problem hiding this comment.
I think that's ok. This is all pretty experimental at this point so having to change features for a new release seems reasonable. We can document it in the release notes.
There was a problem hiding this comment.
I flipped it around.
crates/wasi-nn/src/backend/mod.rs
Outdated
| @@ -2,7 +2,7 @@ | |||
| //! this crate. The `Box<dyn ...>` types returned by these interfaces allow | |||
| //! implementations to maintain backend-specific state between calls. | |||
|
|
|||
| #[cfg(feature = "onnx")] | |||
| #[cfg(any(feature = "onnx", feature = "onnx-local-rt"))] | |||
There was a problem hiding this comment.
If you flip the features around, then these #cfgs no longer need to check both cases, right?
There was a problem hiding this comment.
Yes, no need to check both cases now.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@yanwucai, it looks like this failure is due to some CI job that now needs the |
This change adds a new Cargo feature to allow the users to use a custom build of ONNX Runtime.
This will also help the users who need to remove some crypto-related dependencies imported by the download-binaries feature of ort.