-
Notifications
You must be signed in to change notification settings - Fork 23
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
Precision issue fix #512
Precision issue fix #512
Conversation
@@ -144,7 +144,7 @@ void BasicBackend::PopulateConfigValue(ov::AnyMap& device_config) { | |||
device_config.emplace(ov::hint::inference_precision("f32")); | |||
} | |||
if (global_context_.precision_str.find("ACCURACY") != std::string::npos && | |||
global_context_.device_type == "GPU") { | |||
global_context_.device_type.find("GPU") != std::string::npos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
U should also check for GPU.0 and GPU.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change LGTM. it should handle any '.x' values after 'GPU' (GPU, GPU.0, GPU.1)..
Our current logic would fail for OV 2025.0.
@jatinwadhwa921 can you also please update line 148 to:
if (((OPENVINO_VERSION_MAJOR == 2024) && (OPENVINO_VERSION_MINOR > 1)) || (OPENVINO_VERSION_MAJOR > 2024))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
da4a8fc
to
c557a94
Compare
@@ -144,7 +144,7 @@ void BasicBackend::PopulateConfigValue(ov::AnyMap& device_config) { | |||
device_config.emplace(ov::hint::inference_precision("f32")); | |||
} | |||
if (global_context_.precision_str.find("ACCURACY") != std::string::npos && | |||
global_context_.device_type == "GPU") { | |||
global_context_.device_type.find("GPU") != std::string::npos) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
global_context_.device_type == "GPU") { | ||
if (global_context_.OpenVINO_Version.at(0) >= 2024 && global_context_.OpenVINO_Version.at(1) >= 1) { | ||
global_context_.device_type.find("GPU") != std::string::npos) { | ||
if (((OPENVINO_VERSION_MAJOR == 2024) && (OPENVINO_VERSION_MINOR > 1)) || (OPENVINO_VERSION_MAJOR > 2024)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jatinwadhwa921
I think we should not be using "OPENVINO_VERSION_MAJOR" etc. to check the version
Instead use :
"global_context_.OpenVINO_Version.at(0)" etc.
I think the former is a macro available at compile time and we will get warnings during build because the condition evaluates to a constant.
I just built it with this change and got the following build failure on this line (since warnings are treated as errors)
"warning C4127: conditional expression is constant"
Precision issue fix