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.

Sign up for GitHub

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

[GPU] fix property overwritten issue #28209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+6 −2
Open

[GPU] fix property overwritten issue #28209

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions src/plugins/intel_gpu/src/runtime/execution_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,16 @@ void ExecutionConfig::apply_user_properties(const cldnn::device_info& info) {
}

// Enable KV-cache compression by default for non-systolic platforms
if (!is_set_by_user(ov::hint::kv_cache_precision) && !info.supports_immad) {
if (!is_set_by_user(ov::hint::kv_cache_precision) &&
internal_properties.find(ov::hint::kv_cache_precision.name()) == internal_properties.end() &&
!info.supports_immad) {
set_property(ov::hint::kv_cache_precision(ov::element::i8));
}

// Enable dynamic quantization by default for non-systolic platforms
if (!is_set_by_user(ov::hint::dynamic_quantization_group_size) && !info.supports_immad) {
if (!is_set_by_user(ov::hint::dynamic_quantization_group_size) &&
internal_properties.find(ov::hint::dynamic_quantization_group_size.name()) == internal_properties.end() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

This change will disable kv_cache_compression and dynamic_group_size default configurations for non-systolic platforms entirely, because this check will never return true, as during property registration all properties are added to internal_properties with default values here.:

openvino/src/plugins/intel_gpu/src/runtime/execution_config.cpp

Line 91 in 82d553e

internal_properties[property.first] = property.second;

As a short-term solution, we can avoid using get_property(ov::hint::dynamic_quantization_group_size) calls at runtime for FC configuration, saving the value at model compilation stage to primitive/implementation or somewhere else
However, the proper solution would be to move these default configurations out of this function entirely and call them only once at the very beginning, either in the constructor or right after config creation

!info.supports_immad) {
set_property(ov::hint::dynamic_quantization_group_size(32));
}

Expand Down
Loading