Skip to content

sycl-exp : unify rope neox/norm #7919

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
Merged
joeatodd merged 4 commits into from
Jun 14, 2024
Merged

sycl-exp : unify rope neox/norm #7919

joeatodd merged 4 commits into from
Jun 14, 2024

Conversation

@joeatodd
Copy link
Contributor

@joeatodd joeatodd commented Jun 13, 2024 •
edited
Loading

  • Self Reported Review Complexity:
    • Review Complexity : Low
    • Review Complexity : Medium
    • Review Complexity : High
  • I have read the contributing guidelines

This PR updates the SYCL backend as per #7634. It also updates ggml_backend_sycl_supports_op to reflect the fact that:

  • rope only works for contiguous arrays
  • concat only works for dim==2

joeatodd added 3 commits June 13, 2024 10:30
@joeatodd
[SYCL] unify rope norm/neox
9b81b57 
As per: #7634

Signed-off-by: Joe Todd <joe.todd@codeplay.com>
@joeatodd
[SYCL] Update unsupported ops
0c0f3f0 
Rope is only supported for contiguous input data.

Concat currently only supports dim=2

Signed-off-by: Joe Todd <joe.todd@codeplay.com>
@joeatodd
Formatting
abd7c7b 
Signed-off-by: Joe Todd <joe.todd@codeplay.com>
@github-actions github-actions bot added the SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language label Jun 13, 2024
@joeatodd joeatodd changed the title sycl : unify rope neox/norm sycl-exp : unify rope neox/norm Jun 13, 2024
@joeatodd
Copy link
Contributor Author

joeatodd commented Jun 13, 2024

@ggerganov the target branch for this PR is codeplay/sycl-main, which is a temporary branch which our team has set up to get CI/testing stability while we make a series of performance improvements. Our intention is that these improvements will make it to main soon after, but the stable branch makes it much easier for us to validate our changes in an otherwise fast moving repo.

We decided to open the branch here, rather than in a fork, so that there's more visibility on our work from other llama.cpp developers. However, I can see that this might cause some confusion relating to duplicate PRs etc.

Would you prefer that we take our codeplay/sycl-main branch to a public fork for our team review process, and then raise PRs here when ready for main?

@ggerganov
Copy link
Member

ggerganov commented Jun 14, 2024

@joeatodd No problem at all - feel free to create the branches here for more visibility

AidanBeltonS
AidanBeltonS reviewed Jun 14, 2024
View reviewed changes
Copy link
Contributor

@AidanBeltonS AidanBeltonS left a comment

Choose a reason for hiding this comment

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

One comment, otherwise changes seem fine

Alcpz
Alcpz reviewed Jun 14, 2024
View reviewed changes
@joeatodd
Replace powf with sycl::pow in ggml-sycl.cpp
ded54b5 
Signed-off-by: Joe Todd <joe.todd@codeplay.com>
@joeatodd joeatodd merged commit b2c8c83 into codeplay/sycl-main Jun 14, 2024
@joeatodd joeatodd deleted the codeplay/unify-rope-sycl branch June 14, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@Alcpz Alcpz Alcpz left review comments

@AidanBeltonS AidanBeltonS AidanBeltonS approved these changes

@OuadiElfarouki OuadiElfarouki OuadiElfarouki approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@joeatodd @ggerganov @Alcpz @AidanBeltonS @OuadiElfarouki