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 : storing strides as number of elements instead of number of bytes #623

Open
slaren opened this issue Nov 27, 2023 · 3 comments
Open
Labels
refactoring Refactoring roadmap Part of a roadmap project

Comments

@slaren
Copy link
Member

slaren commented Nov 27, 2023

Currently, we store the strides between elements of each dimension as a number of bytes in ggml_tensor::nb. In practice, this complicates code because strides always need to be multiplied by the element size, and accessing elements requires first casting the pointers to char *.

I am not sure if there are any cases where we would want a byte stride that isn't a multiple of the element size, as this would mean that the addresses would no longer be aligned to the element size, which is not ok in many platforms. Therefore I think we could simplify the code a bit by storing strides as numbers of elements instead of numbers of bytes.

@ggerganov
Copy link
Member

Yup, I guess it would be an improvement. Probably we can rename nb to ns so that we get errors in all places in the codebase when refactoring this. And leave a comment explaining to 3rd party devs how to update if they are using nb somewhere in their projects

@ggerganov ggerganov added the refactoring Refactoring label Nov 27, 2023
@ggerganov ggerganov changed the title Storing strides as number of elements instead of number of bytes ggml : storing strides as number of elements instead of number of bytes Nov 27, 2023
@ggerganov ggerganov moved this to Todo in ggml : roadmap Nov 27, 2023
@ggerganov ggerganov added the roadmap Part of a roadmap project label Feb 4, 2025
@MarioSieg
Copy link

Are the ggml strides and memory actually row major?
Because when I compare ggml strides to numpy strides,
ggml strides are reversed which looks like column major ordering, but in the ggml source comments it says that tensors are stored in row-major order.
I'ts confusing to convert data between ggml and numpy when the stride layout differs so significantly...

@ggerganov
Copy link
Member

I agree it's confusing but it's too late to change. The ggml data is stored in row-major. The shapes and strides are in reverse compared to python.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring roadmap Part of a roadmap project
Projects
None yet
Development

No branches or pull requests

3 participants