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

[SYCL] Update SYCL upscale operation #7321

Merged
merged 3 commits into from
May 20, 2024
Merged

[SYCL] Update SYCL upscale operation #7321

merged 3 commits into from
May 20, 2024

Conversation

AidanBeltonS
Copy link
Collaborator

Adds upscaling changes to SYCL backend using nearest neighbour interpolation.

Tested on PVC and A100 GPUs all upscale tests are passing.
Implementation comes from ggerganov/ggml#814

@mofosyne mofosyne added Review Complexity : Medium Generally require more time to grok but manageable by beginner to medium expertise level generation quality Quality of model output labels May 16, 2024
@NeoZhangJianyu
Copy link
Collaborator

@AidanBeltonS
Could I know what's the benefit of this solution?

@mofosyne mofosyne added the SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language label May 17, 2024
@ggerganov
Copy link
Owner

@NeoZhangJianyu Here is more info: ggerganov/ggml#814

ggerganov
ggerganov reviewed May 17, 2024
View reviewed changes
ggml-sycl.cpp Outdated
Comment on lines 13994 to 13995
#pragma message("TODO: generalize upscale operator")
#pragma message(" https://github.com/ggerganov/ggml/pull/814")
Copy link
Owner

Choose a reason for hiding this comment

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

These messages are no longer needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching this, I'v removed the messages

@AidanBeltonS
Copy link
Collaborator Author

@AidanBeltonS Could I know what's the benefit of this solution?

Outside of the benefits of being able to support the application in the PR. The primary goal for myself is to get all the tests passing in test-backend-ops. This PR enables the passing of the UPSCALE op tests.

@NeoZhangJianyu
Copy link
Collaborator

@AidanBeltonS Could I know what's the benefit of this solution?

Outside of the benefits of being able to support the application in the PR. The primary goal for myself is to get all the tests passing in test-backend-ops. This PR enables the passing of the UPSCALE op tests.

OK, got it!

NeoZhangJianyu
NeoZhangJianyu approved these changes May 17, 2024
View reviewed changes
Copy link
Collaborator

@NeoZhangJianyu NeoZhangJianyu left a comment

Choose a reason for hiding this comment

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

The CI of upscale are passed on Intel Arc770.

ggml-sycl.cpp Show resolved Hide resolved
@abhilash1910
Copy link
Collaborator

@AidanBeltonS could you sync master for latest changes for CI ?

@abhilash1910 abhilash1910 merged commit 6bf9b66 into ggerganov:master May 20, 2024
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggerganov ggerganov ggerganov left review comments

@abhilash1910 abhilash1910 abhilash1910 approved these changes

@NeoZhangJianyu NeoZhangJianyu NeoZhangJianyu approved these changes

@airMeng airMeng Awaiting requested review from airMeng

Assignees
No one assigned
Labels
generation quality Quality of model output Review Complexity : Medium Generally require more time to grok but manageable by beginner to medium expertise level 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.

None yet
5 participants
@AidanBeltonS @NeoZhangJianyu @ggerganov @abhilash1910 @mofosyne