ggml: add type_size to data view offset calc #777
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit suggests adding the type_size to the calculation of the
offset when creating a view of a tensor.
The motivation for this change is that when I tried calling
`ggml_view_1d` using an offset like this:
```c
struct ggml_tensor * view = ggml_view_1d(ctx, x, 5, 0);
```
Where the `x` tensor has 10 elments with values ranging from 0 to 9,
and requesting a view with 5 elements, starting at offset zero this
works as expected.
But if I try using an offset of 5, with the expectation (which may be
incorrect) that this would produce a view with the last 5 elements of
the tensor `x`:
```c
struct ggml_tensor * view = ggml_view_1d(ctx, x, 5, 5);
```
This does not work as I had expected. This commit contains a test with
the above use case, which hopefully clarifies that I'm trying to do and
perhaps someone can point out what I'm doing wrong, or confirm that
this may be an issue with the current implementation.
Signed-off-by: Daniel Bevenius <[email protected]>
|
The offsets of the views are specified in bytes, and you should do the multiplication yourself in the call to |
Thanks for clarifying this, it was not clear to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit suggests adding the type_size to the calculation of the offset when creating a view of a tensor.
The motivation for this change is that when I tried calling
ggml_view_1dusing an offset like this:Where the
xtensor has 10 elments with values ranging from 0 to 9, and requesting a view with 5 elements, starting at offset zero this works as expected.But if I try using an offset of 5, with the expectation (which may be incorrect) that this would produce a view with the last 5 elements of the tensor
x:This does not work as I had expected. This commit contains a test with the above use case, which hopefully clarifies that I'm trying to do and perhaps someone can point out what I'm doing wrong, or confirm that this may be an issue with the current implementation.