Skip to content

[CPU] Set score_len = 1 if no score_aggregation_window #30682

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

Conversation

zhangYiIntel
Copy link
Contributor

@zhangYiIntel zhangYiIntel commented May 23, 2025

Details:

Tickets:

@zhangYiIntel zhangYiIntel requested review from a team as code owners May 23, 2025 02:30
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label May 23, 2025
Copy link
Contributor

@luo-cheng2021 luo-cheng2021 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@wenjiew wenjiew added this to the 2025.2 milestone May 23, 2025
@@ -2410,7 +2410,7 @@ struct MHAHelper {
int32_t total_kv_len_aligned = 0;
int32_t total_kv_len = 0;
for (int32_t i = 0; i < seq_cout; i++) {
auto score_win_len = score_aggregation_window.ptr<int32_t>()[i];
auto score_win_len = score_aggregation_window ? score_aggregation_window.ptr<int32_t>()[i] : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the spec, when score_aggregation_window is unspecified, the value should be 1.

@@ -2538,7 +2538,7 @@ struct MHAHelper {
score_block_ptr,
reinterpret_cast<DATA_TYPE*>(score),
1,
rnd_up(cur_kv_len, _block_size),
cur_kv_len,
Copy link
Contributor

Choose a reason for hiding this comment

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

NOTE for the change: The score_output is aligned to 64 bytes(aka 16 floats), rnd_up(cur_kv_len, _block_size) will align to 32 floats then 2 threads may write to same address and make random wrong result. cvt_add already handles tails so the padding may be removed.

@zhangYiIntel zhangYiIntel changed the title [CPU]set score_len = 0 if no score_aggregation_window [CPU]set score_len = 1 if no score_aggregation_window May 26, 2025
@maxnick maxnick added this pull request to the merge queue May 26, 2025
@maxnick maxnick changed the title [CPU]set score_len = 1 if no score_aggregation_window [CPU] Set score_len = 1 if no score_aggregation_window May 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 26, 2025
@zhangYiIntel zhangYiIntel enabled auto-merge May 26, 2025 15:25
@zhangYiIntel zhangYiIntel added this pull request to the merge queue May 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2025
@zhangYiIntel zhangYiIntel added this pull request to the merge queue May 27, 2025
Merged via the queue into openvinotoolkit:releases/2025/2 with commit 7751623 May 27, 2025
215 of 219 checks passed
@zhangYiIntel zhangYiIntel deleted the yi3/fix_score_agg_win_25_2 branch May 27, 2025 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin Code Freeze
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants