open_vision_capsules icon indicating copy to clipboard operation
open_vision_capsules copied to clipboard

Clean up `get_all_devices` api to clear up confusion over OpenVINO "GPU" and Tensorflow "GPU"

Open apockill opened this issue 3 years ago • 9 comments

Now that some implementations of the OpenVisionCapsules spec are integrating support for loading OpenVINO models onto integrated GPU's, there's a problem of name collisions from the device_mapping.get_all_devices() function. Curreuntly it returns all devices discovered by Tensorflow and OpenVINO combined.

apockill avatar May 24 '21 23:05 apockill

https://github.com/opencv/open_vision_capsules/blob/cbd8bd9a390f407f8971562eaa129ccbf352da67/vcap/vcap/device_mapping.py#L13

Instead of returning List[str] it could return List[Tuple[DeviceProvider, str]], where DeviceProvider is an Enum, with values TENSORFLOW and OPENVINO, and perhaps others in the future.

I'm also open to major overhauls, if anyone has any clever ideas.

apockill avatar May 24 '21 23:05 apockill

If it's an enum, would that prevent users from implementing their own DeviceMappers that map to their own hardware, since they can't add to the enum at runtime? I'm not sure if that's something anyone could do in practice though.

velovix avatar May 26 '21 04:05 velovix

it could return List[DeviceProvider, str]

typing.List only supports homogeneous types (i.e. takes only one argument). Perhaps you meant Dict[DeviceProvider, str]? But this would limit you to only two devices, so maybe Dict[DeviceProvider, Set[str]] would be better?

BryceBeagle avatar May 26 '21 14:05 BryceBeagle

That was a typo, oops! I actually meant List[Tuple[DeviceProvider, str]], because the filterer would likely want to iterate over the provider/device_name pairings. (Edited original post)

The current device mapper filters expect a list of device_names, and the workflow is to just iterate and filter anything that isn't compatible and return the compatible devices.

Another option would be to make a DeviceType dataclass, which has a Provider parameter, so basically:


class Provider(Enum)
	TENSORFLOW = "tensorflow"
    OPENVINO = "openvino"
    ...
    future providers

@dataclass
class DeviceType:
    provider: Provider
    name: str

def get_all_devices() -> List[DeviceType]:
    ...

Thoughts on this?

apockill avatar May 26 '21 17:05 apockill

My vote is for the dataclass :+1:.

Could (should?) get_all_devices return Set[DeviceType]? (you'd have to mark DeviceType as frozen so it can be hashed). Does the order matter?

BryceBeagle avatar May 26 '21 18:05 BryceBeagle

Technically get_all_devices operates with sets internally, then returns a list. So yes, it could and probably should return a set.

Order doesn't matter.

apockill avatar May 26 '21 19:05 apockill

Alex and I talked about a possible design for this where vcap-utils creates Provider classes for TensorFlow and OpenVINO, then registers them with vcap under a string name. Then, the list of devices would be keyed by that string name. This allows other applications, including BrainFrame, to implement their own custom Providers and let capsules depend on them.

velovix avatar May 26 '21 21:05 velovix

Why key them by string name instead of something higher-level?

BryceBeagle avatar May 26 '21 22:05 BryceBeagle

Something higher level would be fine, I just don't think it should be an enum because that would prevent application developers from adding their own providers.

velovix avatar May 26 '21 22:05 velovix