-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[CPU] Enable u8 kv cache by default #27454
[CPU] Enable u8 kv cache by default #27454
Conversation
...tel_cpu/tests/functional/custom/subgraph_tests/src/common/concat_transpose_sdp_transpose.cpp
Show resolved
Hide resolved
@@ -411,6 +412,9 @@ void Config::readProperties(const ov::AnyMap& prop, const ModelType modelType) { | |||
if (!fcDynamicQuantizationGroupSizeSetExplicitly) { | |||
fcDynamicQuantizationGroupSize = 0; | |||
} | |||
if (!kvCachePrecisionSetExplicitly) { |
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.
why not set kvCachePrecision
to u8
here ? To make the kvCachePrecision
compatible in ACL platform ? If so, it's better to left a comment here to explain this.
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.
Setting the kvCachePrecision
in constructor should be more clearer and consistent with other properties such as DynamicQuantizationGroup
, so my suggestion is to keep them in current style.
47823da
to
16578fe
Compare
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, thanks!
### Changes explicitly disable kv cache compression to u8, f16 precision is used instead. ### Reason for changes PTWC nightly has a different metrics (ticket 157594). It happens, because since openvinotoolkit/openvino#27454 KV Cache compressed to u8 by default and it affects accuracy of fp32 models (ticket 157571). Propose using kv cache in the f16 in order to handle issues in nncf rather than in ov (there's still an open issue with kv cache compression, and it can be modified in the nearest future) ### Related tickets 157571 157594 ### Tests - [x] openvino-nightly/job/post_training_weight_compression/56 ![image](https://github.com/user-attachments/assets/0772a8e5-0f92-4f53-8ac0-e16841bd8193) - [x] https://github.com/openvinotoolkit/nncf/actions/runs/11934079602 - [x] job/weekly/job/openvino-nightly/job/test_examples/77
### Details: - *Enable u8 kv cache by default* - *...* ### Tickets: - *[152621](https://jira.devtools.intel.com/browse/CVS-152621)*
### Details: - *Port for enabling u8 kv cache by default in #27454* ### Tickets: - *[152621](https://jira.devtools.intel.com/browse/CVS-152621)* --------- Co-authored-by: Vladislav Golubev <[email protected]>
Details:
Tickets: