Skip to content

fix build break by iq1s#6010

Closed
NeoZhangJianyu wants to merge 3 commits intoggml-org:masterfrom
NeoZhangJianyu:fix_build_break_iq1s
Closed

fix build break by iq1s#6010
NeoZhangJianyu wants to merge 3 commits intoggml-org:masterfrom
NeoZhangJianyu:fix_build_break_iq1s

Conversation

@NeoZhangJianyu
Copy link
Copy Markdown
Contributor

fix the build break issue created by "Add q3_s and q1_s"

Comment thread ggml-sycl.cpp
Comment thread ggml-sycl.cpp
Comment thread ggml-sycl.cpp
return d * sumi;
int sumi1 = 0, sumi2 = 0, sumi3 = 0, sumi4 = 0;
const uint8_t h1 = bq1->scales[2*ib32+0];
const uint8_t h2 = bq1->scales[2*ib32+1];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const uint8_t h2 = bq1->scales[2*ib32+1];
const uint8_t h2 = bq1->qh[2*ib32+1];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I restore the structure with "scales"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should not restore scales - the data structure was changed to improve the quality of IQ1_S. The kernels should be adapted to follow the CUDA implementation

Does this even pass the tests when you run test-backend-ops?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, it can pass with CI in my local test.
But, I want to abandon this PR. base is changed more frequently. :<

We need to fix the SYCL backend build as soon.
Follow CUDA code is not important, if SYCL backend work well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, if you want, you can close the PR

Make sure to take a look at #6014 when you get to this

@NeoZhangJianyu
Copy link
Copy Markdown
Contributor Author

I'd like to restore SYCL backend build as soon as possible.
To avoid breaking CI.

@NeoZhangJianyu
Copy link
Copy Markdown
Contributor Author

abandon due to this issue has been fixed by other PR.

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.

3 participants