intel / openvino-rs

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

Unable to retrieve core properties #122

Closed pnehrer closed 2 months ago

pnehrer commented 2 months ago

Retrieving a non-device specific property from core results in a "general error". E.g., the following unit test fails:

    #[test]
    fn test_get_supported_properties() {
        let core = Core::new().unwrap();
        let supported_properties = core.get_property(PropertyKey::SupportedProperties);
        assert!(supported_properties.is_ok());
    }

Retrieving device-specific properties appear to be working (e.g., for CPU).

rahulchaphalkar commented 2 months ago

Thanks for bringing this up! Looks like official get_property (and set_property) function documented takes device_name as a parameter, so we should probably get rid of our bindings which allow no device name to be passed.

Interestingly, openvino does have 3 non-device specific properties which can be returned successfuly w/o specifying device name (source). In the current state, if you pass either EnableMmap, CacheDir, or ForceTbbTerminate, your test actually does pass.

However, since this is not documented, we should probably remove the option to not pass a device. @abrown what do you think?

abrown commented 2 months ago

Thanks for bringing this up! Looks like official get_property (and set_property) function documented takes device_name as a parameter, so we should probably get rid of our bindings which allow no device name to be passed.

We do have Core::get_device_property, so let's change the parameter there to be an enum that contains only the valid property keys.

Interestingly, openvino does have 3 non-device specific properties which can be returned successfuly w/o specifying device name (source). In the current state, if you pass either EnableMmap, CacheDir, or ForceTbbTerminate, your test actually does pass.

How about this: let's add an enum which contains only the three variants allowed by Core::get_property and use that type for the parameter.


[edit: @rahulchaphalkar and I talked and none of the above applies; let's wait for his PR]