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

Jump to bottom

[GPU] Support rope for glm4v #27545

Merged
merged 15 commits into from
Nov 22, 2024
Merged

[GPU] Support rope for glm4v #27545

merged 15 commits into from
Nov 22, 2024

Conversation

zhaohb
Copy link
Contributor

@zhaohb zhaohb commented Nov 14, 2024 •
edited by peterchen-intel
Loading

support rope kernel for glm4v

Tickets: CVS-157422

@zhaohb zhaohb requested a review from a team as a code owner November 14, 2024 02:46
@zhaohb zhaohb requested review from itikhono and removed request for a team November 14, 2024 02:46
@github-actions github-actions bot added the category: transformations OpenVINO Runtime library - Transformations label Nov 14, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Nov 14, 2024
@zhaohb zhaohb changed the title Support rope for glm4v [GPU] Support rope for glm4v Nov 14, 2024
@zhaohb zhaohb marked this pull request as draft November 14, 2024 02:53
@zhaohb zhaohb marked this pull request as ready for review November 14, 2024 07:15
@sammysun0711
Copy link
Collaborator

@ahnyoung-paul, @isanghao @yeonbok @itikhono, could you please kindly review this PR. Thanks!

zhangYiIntel
zhangYiIntel requested changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@zhangYiIntel zhangYiIntel left a comment

Choose a reason for hiding this comment

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

Please add test for this new pattern at
src/common/transformations/tests/common_optimizations/fuse_rotary_positional_embeddings.cpp

@p-durandin
Copy link
Contributor

build_jenkins

@zhaohb
Copy link
Contributor Author

zhaohb commented Nov 18, 2024

Please add test for this new pattern at src/common/transformations/tests/common_optimizations/fuse_rotary_positional_embeddings.cpp

Hi @zhangYiIntel Test cases have been added.

@sammysun0711 sammysun0711 removed the ExternalPR External contributor label Nov 18, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Nov 19, 2024
zhangYiIntel
zhangYiIntel approved these changes Nov 19, 2024
View reviewed changes
Copy link
Contributor

@zhangYiIntel zhangYiIntel left a comment

Choose a reason for hiding this comment

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

LGTM, please go head to make CI green

@sammysun0711
Copy link
Collaborator

build_jenkins

@FionaZZ92
Copy link
Contributor

@itikhono Can you please help to review and check able to merge.

@sammysun0711 sammysun0711 removed the ExternalPR External contributor label Nov 20, 2024
@sammysun0711
Copy link
Collaborator

build_jenkins

@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Nov 20, 2024
@sammysun0711
Copy link
Collaborator

build_jenkins

@itikhono
Copy link
Contributor

itikhono commented Nov 21, 2024 •
edited
Loading

the changes LGTM, I will double check it with glm4 models and merge

@itikhono itikhono enabled auto-merge November 22, 2024 09:14
@sammysun0711
Copy link
Collaborator

build_jenkins

@itikhono
Copy link
Contributor

we have checked the updated transformation with @sammysun0711
And it looks like the transformation works on GPU with glm4 model but it doesn't work on CPU.
This is a common transformation so we expect it works on both GPU and CPU.
For CPU we are going to create a separate ticket and investigate the case.

This is not a blocker for this PR.

@sammysun0711
Copy link
Collaborator

Thanks @itikhono confirmation, we will follow up RoPE fusion for CPU part in separate ticket.

@itikhono itikhono added this pull request to the merge queue Nov 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 22, 2024
@sammysun0711 sammysun0711 added this pull request to the merge queue Nov 22, 2024
Merged via the queue into openvinotoolkit:master with commit edcd4d8 Nov 22, 2024
170 checks passed
sammysun0711 added a commit to zhaohb/openvino that referenced this pull request Nov 26, 2024
@zhaohb @peterchen-intel @sammysun0711
[GPU] Support rope for glm4v (openvinotoolkit#27545)
c4a18db 
support rope kernel for glm4v

**Tickets:** CVS-157422

---------

Co-authored-by: Chen Peter <[email protected]>
Co-authored-by: Xiake Sun <[email protected]>
NishantPrabhuFujitsu pushed a commit to NishantPrabhuFujitsu/openvino that referenced this pull request Nov 26, 2024
@zhaohb @peterchen-intel
@sammysun0711 @NishantPrabhuFujitsu
[GPU] Support rope for glm4v (openvinotoolkit#27545)
c91ece9 
support rope kernel for glm4v

**Tickets:** CVS-157422

---------

Co-authored-by: Chen Peter <[email protected]>
Co-authored-by: Xiake Sun <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Nov 26, 2024
@zhaohb @peterchen-intel @sammysun0711
[GPU] Support rope for glm4v (#27545) (#27713)
0515bd3 
support rope kernel for glm4v

**Tickets:** CVS-157422

Co-authored-by: Chen Peter <[email protected]>
Co-authored-by: Xiake Sun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhangYiIntel zhangYiIntel zhangYiIntel approved these changes

@itikhono itikhono itikhono approved these changes

@isanghao isanghao Awaiting requested review from isanghao

@ahnyoung-paul ahnyoung-paul Awaiting requested review from ahnyoung-paul

@yeonbok yeonbok Awaiting requested review from yeonbok

@ceciliapeng2011 ceciliapeng2011 Awaiting requested review from ceciliapeng2011

@riverlijunjie riverlijunjie Awaiting requested review from riverlijunjie

Assignees

@dorloff dorloff

@andrei-kochin andrei-kochin

Labels
category: transformations OpenVINO Runtime library - Transformations ExternalPR External contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@zhaohb @sammysun0711 @p-durandin @FionaZZ92 @itikhono @zhangYiIntel @dorloff @andrei-kochin @sys-openvino-ci @peterchen-intel