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

feat: adds ggml_pad_ext to allow prefix padding #864

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

balisujohn
Copy link
Contributor

@balisujohn balisujohn commented Jun 18, 2024

modifies ggml_pad so you can specify beginning and end padding for all 4 dimensions of a tensor. Previously, padding could only be added at the end of the first 2 dimensions. The padding is still constant padding with a value of zero, as before.

Adds new test file test-pad.cpp

@balisujohn balisujohn marked this pull request as draft June 18, 2024 08:44
@balisujohn
Copy link
Contributor Author

I forgot that ggml_pad actually supports 4 dimensions as-is, I will add the support for pre-fix padding in all 4 dimensions, so marking as a draft for now.

@balisujohn
Copy link
Contributor Author

balisujohn commented Jun 19, 2024

Alright so it supports up to 4d prefix and postfix padding with the CPU version. for cuda it supports up to 3d prefix and postfix padding (the cuda pad kernel was previously limited to 3d tensors and I didn't remove this limitation). Two of the tests in test-pad.cpp use 4d tensors so they would need to be commented out to run the tests in cuda mode, though I verified all the other tests work with the cuda kernel.

This modification leaves the arguments of ggml_pad as-is, and adds ggml_pad_ext which requires the desired prefix and postfix length to be specified for all 4 dimensions.

Ready for review :^)

@balisujohn balisujohn marked this pull request as ready for review June 19, 2024 18:21
@balisujohn balisujohn changed the title feat: extends ggml_pad to allow prefix padding feat: adds ggml_pad_ext to allow prefix padding Jun 19, 2024
@balisujohn
Copy link
Contributor Author

balisujohn commented Jul 2, 2024

(I noticed I need to change some variable names in test-pad.cpp and there are similar minor fixes needed in the pad_reflect_1d PR)

@balisujohn
Copy link
Contributor Author

this and pad_reflect_1d are ready for review again :^)

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.

2 participants