Support loading non-string models from memory #107
Conversation
crates/openvino/src/core.rs
Outdated
| model_str: &str, | ||
| weights_buffer: &Tensor, | ||
| model_str: &[u8], | ||
| weights_buffer: T, |
There was a problem hiding this comment.
I propose we move back to the Option<&Tensor> like you had; it's a bit more straightforward and avoids potential concerns with code size, etc.
|
Oh, and one other thing: the CI failures are not due to this change. I think you will have to rebase to see CI turn green. |
I think this is a function that isn't in 2022.3.0, which is what seems to be failing to link. Some more recent version jobs do succeed before this one cancels the rest. |
... and as a result, I can remove the unit test if that helps. |
|
Ah, I see: @rahulchaphalkar and I noticed a failure in the Windows builds due to the missing function and I chalked it up to some difference between the Windows and Linux releases. But actually we were never exercising the function so it would have failed if someone actually tried it. Given that, I propose we keep the test and remove the following step in CI since we just can't support those versions with this function: openvino-rs/.github/workflows/main.yml Lines 28 to 32 in 3a404ee With that out of the way, I think this should be good to go 🤞 |
|
Would you still want to keep the "oldest supported version" with apt and just pull it up to 2023.2.0, instead of removing that combo altogether? |
Well, we have 2023.2.0 up above in the non-APT build so I'm not too worried about it. |
We've run into an issue with
Core::read_model_from_buffer-- the wrapper assumes that the in-memory model is a valid string. However, non-native models, such as ONNX, aren't valid strings. Attempting to load an ONNX model results in an error creating a CString due to non-terminating null characters.We propose to change the method interface to support arbitrary byte models rather than just strings.
Furthermore, we propose to make the weight tensor an optional argument, as it doesn't apply to non-native models.