Skip to content

Commit

Permalink
Check version; fail if pre-2024.2 (#149)
Browse files Browse the repository at this point in the history
* Check version; fail if pre-2024.2

In #143, we discovered that upstream OpenVINO had changed the
`ov_element_type_e` enum in a backwards-incompatible way. This means
that a mismatch between the Rust bindings (based on the C headers) and
the shared library will result in tensors being created with the wrong
type (e.g., `U64` instead of `U8`). To avoid this situation, this change
reads the OpenVINO library version at runtime (e.g., every time a `Core`
is created) and fails with an error if the version is older than 2024.2,
when the breaking change was introduced.

The effect of merging this change would be that newer bindings will fail
when used with older libraries, which could be problematic for users.
Users could always use an older version of the library (e.g., `v0.7.*`)
instead. And the alternative, letting users create tensors of the wrong
type, seems like it would open up the door to a slew of reported issues.

* Add documentation to helper functions

* Update CI versions: library + OS

This updates CI to test only the latest three OpenVINO versions, since
older versions will no longer be accessible due to the new "breaking
change version check." It also updates the OS version of the GitHub
runner for good measure.

* ci: specify ubuntu-24.04

* ci: roll back OS version

OpenVINO has not published packages for `ubuntu-24.04`.
  • Loading branch information
abrown authored Oct 14, 2024
1 parent 61d73c4 commit 2af130a
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
# found ("dyld: Library not loaded; '@rpath/libopenvino.2310.dylib'"). See
# https://github.com/abrown/openvino-rs/actions/runs/6423141936/job/17441022932#step:7:154
os: [ubuntu-20.04, ubuntu-22.04, windows-latest]
version: [2023.2.0, 2024.1.0, 2024.4.0]
version: [2024.2.0, 2024.3.0, 2024.4.0]
apt: [false]
# We also spot-check that things work when installing from APT by adding to the matrix: see
# https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#expanding-or-adding-matrix-configurations
Expand Down
53 changes: 48 additions & 5 deletions crates/openvino-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,54 @@ pub use generated::*;
pub mod library {
use std::path::PathBuf;

// Include the definition of `load` here. This allows hiding all of the "extra" linking-related
// functions in the same place, without polluting the top-level namespace (which should only
// contain foreign functions and types).
#[doc(inline)]
pub use super::generated::load;
/// When compiled with the `runtime-linking` feature, load the function definitions from a
/// shared library; with the `dynamic-linking` feature, this function does nothing since the
/// library has already been linked.
///
/// # Errors
///
/// When compiled with the `runtime-linking` feature, this may fail if the `openvino-finder`
/// cannot discover the library on the current system. This may also fail if we link to a
/// version of OpenVINO that is too old for these Rust bindings: the upstream library changed
/// the `ov_element_type_e` enum in a backwards-incompatible way in v2024.2, meaning users would
/// unintentionally use the wrong type when creating tensors (see [#143]).
///
/// [#143]: https://github.com/intel/openvino-rs/issues/143
pub fn load() -> Result<(), String> {
super::generated::load()?;
let version = get_version()?;
if is_pre_2024_2_version(&version) {
return Err(format!("OpenVINO version is too old (see https://github.com/intel/openvino-rs/issues/143): {version}"));
}
Ok(())
}

/// Retrieve the OpenVINO library's version string.
fn get_version() -> Result<String, String> {
use super::generated::{
ov_get_openvino_version, ov_status_e, ov_version_free, ov_version_t,
};
let mut ov_version = ov_version_t {
buildNumber: std::ptr::null(),
description: std::ptr::null(),
};
let code = unsafe { ov_get_openvino_version(&mut ov_version) };
if code != ov_status_e::OK {
return Err(format!("failed to get OpenVINO version: {code:?}"));
}
let c_str_version = unsafe { std::ffi::CStr::from_ptr(ov_version.buildNumber) };
let version = c_str_version.to_string_lossy().into_owned();
unsafe { ov_version_free(std::ptr::addr_of_mut!(ov_version)) };
Ok(version)
}

/// Parse the version string and return true if it is older than 2024.2.
fn is_pre_2024_2_version(version: &str) -> bool {
let mut parts = version.split(['.', '-']);
let year: usize = parts.next().unwrap().parse().unwrap();
let minor: usize = parts.next().unwrap().parse().unwrap();
year < 2024 || (year == 2024 && minor < 2)
}

/// Return the location of the shared library `openvino-sys` will link to. If compiled with
/// runtime linking, this will attempt to discover the location of a `openvino_c` shared library
Expand Down
10 changes: 0 additions & 10 deletions crates/openvino/tests/memory-safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,13 @@
//! be sure that we do the right thing on this side of the FFI boundary.

mod fixtures;
mod util;

use fixtures::mobilenet as fixture;
use openvino::{Core, DeviceType, ElementType, Shape, Tensor};
use std::fs;
use util::is_version_pre_2024_2;

#[test]
fn memory_safety() -> anyhow::Result<()> {
// OpenVINO 2024.2 changed the order of the `ov_element_type_e` enum, breaking compatibility
// with older versions. Since we are using 2024.2+ bindings here, we skip this test when
// using older libraries.
if is_version_pre_2024_2() {
eprintln!("> skipping test due to pre-2024.2 OpenVINO version");
return Ok(());
}

let mut core = Core::new()?;
let xml = fs::read_to_string(fixture::graph())?;
let weights = fs::read(fixture::weights())?;
Expand Down
10 changes: 0 additions & 10 deletions crates/openvino/tests/setup.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
//! These tests demonstrate how to setup OpenVINO networks.

mod fixtures;
mod util;

use fixtures::alexnet as fixture;
use openvino::{Core, ElementType, Shape, Tensor};
use std::fs;
use util::is_version_pre_2024_2;

#[test]
fn read_network() {
Expand All @@ -25,14 +23,6 @@ fn read_network() {

#[test]
fn read_network_from_buffers() {
// OpenVINO 2024.2 changed the order of the `ov_element_type_e` enum, breaking compatibility
// with older versions. Since we are using 2024.2+ bindings here, we skip this test when
// using older libraries.
if is_version_pre_2024_2() {
eprintln!("> skipping test due to pre-2024.2 OpenVINO version");
return;
}

let mut core = Core::new().unwrap();
let graph = fs::read(&fixture::graph()).unwrap();
let weights = {
Expand Down
13 changes: 0 additions & 13 deletions crates/openvino/tests/util.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
#![allow(dead_code)] // Not all functions are used by each test.

use core::cmp::Ordering;
use float_cmp::{ApproxEq, F32Margin};
use openvino::version;

/// A structure for holding the `(category, probability)` pair extracted from the output tensor of
/// the OpenVINO classification.
Expand Down Expand Up @@ -82,13 +79,3 @@ pub const DEFAULT_MARGIN: F32Margin = F32Margin {

/// A helper type for manipulating lists of results.
pub type Predictions = Vec<Prediction>;

/// OpenVINO's v2024.2 release introduced breaking changes to the C headers, upon which this crate
/// relies. This function checks if the running OpenVINO version is pre-2024.2.
pub fn is_version_pre_2024_2() -> bool {
let version = version();
let mut parts = version.parts();
let year: usize = parts.next().unwrap().parse().unwrap();
let minor: usize = parts.next().unwrap().parse().unwrap();
year < 2024 || (year == 2024 && minor < 2)
}

0 comments on commit 2af130a

Please sign in to comment.