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_conv_2d input shape #954

Open
StudyingLover opened this issue Sep 8, 2024 · 4 comments
Open

ggml_conv_2d input shape #954

StudyingLover opened this issue Sep 8, 2024 · 4 comments

Comments

@StudyingLover
Copy link

StudyingLover commented Sep 8, 2024

I am a freshman about GGML, when I read the source code, I notice that the function struct ggml_tensor * ggml_conv_2d

ggml/src/ggml.c

Line 6889 in 10e83a4

struct ggml_tensor * ggml_conv_2d(

from the comment, wo can know the input shape of a is [OC,IC, KH, KW] and shape of b is [N, IC, IH, IW] ,but in the function ggml_im2col(ctx, a, b, s0, s1, p0, p1, d0, d1, true, a->type)

ggml/src/ggml.c

Line 6819 in 10e83a4

GGML_ASSERT(a->ne[2] == b->ne[2]);

we assert GGML_ASSERT(a->ne[2] == b->ne[2]);, which means KH== IH, this not right I think.

I’m not sure if I understood correctly. If I misunderstood, please correct me. If there is an error in the comments, I am willing to submit a PR to fix it.

@bssrdf
Copy link
Contributor

bssrdf commented Sep 8, 2024

The comments (for some reason) use Pytorch dimension convention which is opposite to what GGML has internally.
a->ne[2] and b->ne[2] are both IC, the 3rd dimension ( 2nd in Pytorch).

@StudyingLover
Copy link
Author

StudyingLover commented Sep 9, 2024 via email

@JohannesGaessler
Copy link
Collaborator

When I recently worked on IM2COL I didn't look at or try to understand the comments but the dimensions of a and b for a two-dimensional convolution should be in order: width, height, number of input channels, and batch size (I'm not 100% sure about the order of the first two). In examples/mnist-common.cpp there are asserts that let you infer at least the externally visible tensor shapes.

@StudyingLover
Copy link
Author

yes,I used examples/mnist-common.cpp to help with understanding, but I believe that having correct comments is also very important.

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

No branches or pull requests

3 participants