Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary device queries #620

Open
wants to merge 4 commits into
base: ovep-develop
Choose a base branch
from

Conversation

nproshun
Copy link

@nproshun nproshun commented Mar 18, 2025

Description

This PR introduces the following changes:

  1. ParseDeviceType is rewritten so that it calls GetAvailableDevices only in case of custom device. If the device prefix is one of (NPU, CPU, GPU) or it's a composite device consisting of such devices, check for availability is deferred until the call to OpenVINOExecutionProvider constructor. Parsing logic is rewritten.
  2. A string argument is added to the OVCore::GetAvailableDevices function, which is empty by default. If it's not empty, only devices of the given type (e.g. GPU or NPU) are queried using get_property function instead of get_available_devices.
  3. OpenVINOExecutionProvider constructor is rewritten so that it only checks the availability of devices of the given type. Also, check for composite devices are added.
  4. ParsePrecision is rewritten to address parsing issues.

Motivation and Context

Removes one unnecessary call to GetAvailableDevices and changes the second one to only query the devices of specified type.
Fixes https://jira.devtools.intel.com/projects/HAFP/issues/HAFP-3018

@nproshun nproshun marked this pull request as ready for review March 18, 2025 11:31
Copy link

@mklimenk mklimenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall with a couple of small remarks, thanks!

@nproshun
Copy link
Author

A diagram for easier understanding of ParseDeviceType

flowchart TD
    A[Start] --> B{Check if device_type is provided by user in provider_options}
    B -->|Yes| C[Get selected_device from provider_options]
    B -->|No| D{Check default device configuration}
    C --> E{Check if selected_device starts with HETERO/MULTI/BATCH/AUTO}
    E -->|Yes| F[Split devices and add to devices_to_check]
    E -->|No| G[Add selected_device to devices_to_check]
    D -->|OPENVINO_CONFIG_CPU/GPU/NPU| H[Set selected_device to CPU/GPU/NPU]
    D -->|OPENVINO_CONFIG_HETERO/MULTI/AUTO| K[Set selected_device to DEVICE_NAME and split devices]
    H --> L[Return selected_device]
    K --> M[Add sub-devices to devices_to_check]
    G --> N[Remove _* and .* suffixes and check each device in devices_to_check]
    F --> N
    M --> N
    N --> O{"Is device supported by default (e.g. it's NPU, CPU or GPU)?"}
    O -->|Yes| P[Return selected_device]
    O -->|No| Q["Check if device (with .idx) is available by calling GetAvailbleDevices"]
    Q -->|Available| P
    Q -->|Not Available| R[Throw error]
Loading

std::string device_prefix = device;
int device_idx = 0;
// Get the index and remove the index from device_prefix
if (auto delimit = device_prefix.find("."); delimit != std::string::npos) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The index is used to identify the id's of Discrete and Integrated GPU . For eg GPU.0, GPU.1. We should check with the device id to check if the specified device is available

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is done on line 100 in the same file, and it uses device, not device_prefix, so the index is preserved. There was a bug in this line, but I've fixed it now.

(device_type.find("MULTI:") == 0) ||
(device_type.find("BATCH:") == 0) ||
(device_type.find("AUTO:") == 0)) {
if (!provider_options.contains(option_name)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not mandatory option to be set. We go with default precision for CPU = FP32, GPU=FP16, NPU=FP16

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've only made it mandatory for composite devices, like MULTI:CPU,GPU,NPU. I can also set some default for this case, but I'm not sure what should I choose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants