intel / openvino-rs

Rust bindings for OpenVINO™
Apache License 2.0
80 stars 23 forks source link

API 2.0 and 2024.0 compatibility #92

Open BTOdell opened 4 months ago

BTOdell commented 4 months ago

This PR is based on #91 but with additional changes, bug fixes, and feature improvements.

Changes

BTOdell commented 4 months ago

I reverted the result assertion in the detect_inception test to see what the actual values were:

assertion `left == right` failed
  left: [Result(22, 67.0), Result(15, 59.0), Result(1, 1.0), Result(8, 1.0), Result(12, 1.0)]
 right: [Result(15, 59.0), Result(1, 1.0), Result(8, 1.0), Result(12, 1.0), Result(16, 0.9939936)]

The second value in the Result is supposed to be a probability, so I'm not sure why values greater than 1.0 are showing up at all. Maybe the output needs a manual post-processing step to throw away outliers?

BTOdell commented 4 months ago

Also, on my AMD laptop, the classify_mobilenet test is also failing with:

Expected probability 0.7134405 but found 0.6203276 (outside of default margin of error)
abrown commented 4 months ago

@BTOdell, #91 has merged; do you want to rebase this on that? Feel free to contact me on Zulip if you want to discuss a strategy for merging this. I would lean toward smaller PRs (except the one-time "jump to v2.0" insanity); how do you feel about breaking this up into a few smaller PRs?

BTOdell commented 4 months ago

Yes, I plan on rebasing this PR at some point. My fork is working for my use case and I'm very busy at the moment, so it will be a while before I can work on this again.

rahulchaphalkar commented 3 months ago

Hi @BTOdell, we're looking forward to the next release of the openvino crate soon, and we were hoping some of the improvements you've made here would be incorporated in that release. Do you want me to cherry-pick some of the changes if possible from this PR, so that we would have a release candidate ready?

BTOdell commented 3 months ago

Yeah, feel free to cherry-pick.

In addition to the items mentioned in the description, I added quite a few functions that were not originally ported in your PR:

  1. Async infer with callback
  2. Property get/set for core and devices
  3. Versions/available_devices query functions
  4. More tensor-accessing functions for InferRequest

I was also having an issue calling variadic C functions due to a bug in the link! macro. I don't know if my "fix" is the proper way to do it, as I'm a noob when it comes to Rust macros. I think I only fixed the runtime linking, but not the dynamic linking (as I don't need it for my use case, but needs to be working for the official crate obviously).

rahulchaphalkar commented 3 months ago

Hi @BTOdell , I've started cherry picking some of the commits, and for few others I need to manually change some code so as to not cause merge conflicts. If you can let me know your email associated with github, I can add you as co-author (cherry-pick did this automatically). This can even be your no-reply* email github provides.