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.

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

IQ1_S: attempt to fix SYCL #6014

Closed
wants to merge 3 commits into from
Closed

IQ1_S: attempt to fix SYCL #6014

wants to merge 3 commits into from

Conversation

ikawrakow
Copy link
Contributor

@ggerganov I see you have a WIP to do this, but as far as I can tell this is for the previous IQ1_S version, so decided to try to fix it myself.

@ggerganov
Copy link
Owner

Thanks for looking into this

Assigning the Intel team for review. AFAICT they are currently focused on resolving some additional problems with the SYCL backend (#6006) and might take them some time to review this, and I don't want to interfere in their work with more PRs (as long as the CI on master is green). Will let them merge this when they think it is appropriate

@NeoZhangJianyu
Copy link
Collaborator

NeoZhangJianyu commented Mar 13, 2024

@ikawrakow @ggerganov
Due to the CI always be broken from yesterday, it's hard to make new PR be passed by CI.

The code of IQ1_S are fixed by several PRs with different solution. It will make this part be complex and easy be with error.
I suggest:

  1. pending the new PRs (which not fix CI) until the CI is restored.
  2. after the CI is restored, please rebased code and test with unit-test locally to make sure IQ1_S work well.

PS: PR (#6006) is cancelled due to I see another PR has fixed the SYCL backend build issue.

@ikawrakow
Copy link
Contributor Author

So, I'm not sure what those PR's that fix IQ1_S are, but what we currently have does not work, and to me it does not look like it ever did (not even before I made the changes to IQ1_S, which this PR is addressing).

No, I don't have a local environment to test the SYCL implementation. But the CI is definitely not failing because of this PR.

@abhilash1910
Copy link
Collaborator

abhilash1910 commented Mar 13, 2024

@ikawrakow We will review this shortly as we close some major fixes. Thanks for your contribution.

@ikawrakow ikawrakow requested review from ggerganov and removed request for abhilash1910 and NeoZhangJianyu March 21, 2024 10:46
@NeoZhangJianyu
Copy link
Collaborator

NeoZhangJianyu commented Apr 7, 2024

@ikawrakow
Based on your previews work, the IQ types in this PR are supported by #6521.
I think it could be closed.

Thank your support!

@mofosyne mofosyne closed this May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants