Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/models/model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,19 +661,18 @@ DeviceInterface* SetProviderSessionOptions(OrtSessionOptions& session_options,
std::optional<uint32_t> config_device_id = resolved_device_filtering.hardware_device_id;
std::optional<uint32_t> config_vendor_id = resolved_device_filtering.hardware_vendor_id;
std::optional<OrtHardwareDeviceType> config_device_type_enum = resolved_device_filtering.hardware_device_type;
// for OpenVINO, use "device_type" in provider_options exclusively if it's provided
// for OpenVINO, fallback to "device_type" in provider_options if no device filtering is specified
std::optional<std::string> config_ov_device_type = std::nullopt;
if (provider_options.name == "OpenVINO") {
if (provider_options.name == "OpenVINO" &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you make the change at config parsing time? So, we update the provider_options.device_filtering_options after parsing the config. And we use that same option for setting the session options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree this would be a cleaner solution. But we need to deprecate the following Intel specific behavior to do so:
As you can see from the code, Intel compares the "device_type" to the "ov_device" in their ep metadata for device selection. This behaves the same if "device_type" is always CPU, GPU or NPU (or any other standard device type in the future). However, Intel also supports setting the ov_device to something like "GPU1" in some versions of their EPs. And use that to select from multiple devices.
IMHO, Intel's old approach is not a very good one since you cannot generate a model with the correct setting if you don't know the CPUx name ahead. And it can be fully replaced by the device filtering APIs. If we can accept deprecation of this behavior, I'll happily remove the OpenVINO spaghetti here and move it forward to config parsing.

!config_device_id.has_value() &&
!config_vendor_id.has_value() &&
!config_device_type_enum.has_value()) {
for (auto& option : provider_options.options) {
if (option.first == "device_type") {
config_ov_device_type = option.second;
}
}
if (config_ov_device_type.has_value()) {
config_device_id = std::nullopt;
config_vendor_id = std::nullopt;
config_device_type_enum = std::nullopt;
} else if (!(config_device_id.has_value() || config_vendor_id.has_value() || config_device_type_enum.has_value())) {
if (!config_ov_device_type.has_value()) {
config_ov_device_type = "CPU";
}
}
Expand Down
Loading