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

ggml : always check bounds on get_rows operations #9354

Merged
merged 1 commit into from
Sep 7, 2024
Merged

Conversation

slaren
Copy link
Member

@slaren slaren commented Sep 7, 2024

Since this seems to be a fairly common problem, promote the bounds checking assert to GGML_ASSERT in get_rows operations so that these errors can be detected in release builds as well. The performance hit should be negligible.

@github-actions github-actions bot added the ggml changes relating to the ggml tensor library for machine learning label Sep 7, 2024
@slaren
Copy link
Member Author

slaren commented Sep 7, 2024

The server test is failing in the embedding test due to an overflow in the positions embedding. I imagine this is because it is trying to evaluate a sequence longer than the maximum sequence length of the model, but I don't know enough about the way embedding models are implemented.

@slaren slaren merged commit e32d081 into master Sep 7, 2024
48 of 52 checks passed
@slaren slaren deleted the sl/get-rows-assert branch September 7, 2024 18:23
@ggerganov ggerganov mentioned this pull request Sep 7, 2024
4 tasks
@slaren slaren mentioned this pull request Sep 12, 2024
4 tasks
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ggml changes relating to the ggml tensor library for machine learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants