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

ttnn.pad does not when support output_shape[-1] is odd #15603

Closed
swimdi opened this issue Nov 27, 2024 · 2 comments
Closed

ttnn.pad does not when support output_shape[-1] is odd #15603

swimdi opened this issue Nov 27, 2024 · 2 comments
Assignees

Comments

@swimdi
Copy link
Contributor

swimdi commented Nov 27, 2024

We convert aten.constant_pad_nd.default to ttnn.pad and it currently does not support cases wheb output_shape[-1] is odd

or it will TT_THROW pad_op.cpp:34: this->output_tensor_shape[3] % 2 == 0 RM padding requires output X dim to be a multiple of 2

These input varation are appeared in models and is related to this constraint

["Tensor<[1, 3, 224, 224]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 96, 112, 112]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 144, 56, 56]> self = ?", "List[int] pad = [1, 2, 1, 2]", "number value = 0.0"],
["Tensor<[1, 240, 28, 28]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 672, 14, 14]> self = ?", "List[int] pad = [1, 2, 1, 2]", "number value = 0.0"],
["Tensor<[1, 3, 240, 240]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 96, 120, 120]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 144, 60, 60]> self = ?", "List[int] pad = [1, 2, 1, 2]", "number value = 0.0"],
["Tensor<[1, 240, 30, 30]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 672, 15, 15]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 3, 260, 260]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 96, 130, 130]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 144, 65, 65]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 288, 33, 33]> self = ?", "List[int] pad = [1, 1, 1, 1]", "number value = 0.0"],
["Tensor<[1, 720, 17, 17]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 3, 300, 300]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 144, 150, 150]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 192, 75, 75]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 288, 38, 38]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 816, 19, 19]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 3, 380, 380]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 144, 190, 190]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 192, 95, 95]> self = ?", "List[int] pad = [2, 2, 2, 2]", "number value = 0.0"],
["Tensor<[1, 336, 48, 48]> self = ?", "List[int] pad = [0, 1, 0, 1]", "number value = 0.0"],
["Tensor<[1, 960, 24, 24]> self = ?", "List[int] pad = [1, 2, 1, 2]", "number value = 0.0"],
@ayerofieiev-tt ayerofieiev-tt transferred this issue from tenstorrent/pytorch2.0_ttnn Dec 2, 2024
@ayerofieiev-tt ayerofieiev-tt changed the title aten.constant_pad_nd.default not support output_shape[-1] is odd ttnn.pad does not when support output_shape[-1] is odd Dec 2, 2024
@jaykru-tt
Copy link
Contributor

I think I have a fix for this that should be landing soon with the fixes for #12896 and #15511. I'll give you a heads up when it's merged.

jaykru-tt added a commit that referenced this issue Dec 14, 2024
### Tickets
- #15511 
- #15603 (90% resolved
with these changes and to be fully resolved in a future PR)
- #12896

### Problem description
ttnn.pad's RM sharded implementation only has support for padding along
the non-width dimensions. The row major implementation additionally is
not fully general with respect to the width dimension, so until now
there are no great options for padding along width. In a future PR
coming tomorrow, I'll add input massaging code to convert to row-major
and shard as needed for input configurations that aren't currently
supported by pad.

### What's changed
- Adds new kernels to support padding along the width dimension.
- For pad operations requiring both NCH and width padding, we use a
fused op using the original height-padding kernels and the new width
kernels.
- The previous point required extensive refactoring to the host code. I
would like eyes on pad.cpp please @yugaoTT @sminakov-tt.
- Also adds a bunch of common utility functions for working with sharded
tensors:
- A function for easily creating sharded memory configs from C++
(analogous to the Python `create_sharded_memory_config` utility function
created by @ntarafdar)
- A function for locating elements of a shard by their coordinates
within the tensor. I've tested this one in the context of this PR, but
it didn't end up being necessary in the final implementation.

### Checklist
- [~] [Post commit CI
passes](https://github.com/tenstorrent/tt-metal/actions/runs/12327681570)
- [x] [Model regression CI testing
passes](https://github.com/tenstorrent/tt-metal/actions/runs/12308045581)
- [x] [Device performance regression CI testing
passes](https://github.com/tenstorrent/tt-metal/actions/runs/12308046347)
- [ ] Blackhole Post commit (if applicable)
- [ ] **(For models and ops writers)** Full [new
models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml)
tests passes
- [ ] New/Existing tests provide coverage for changes

---------

Co-authored-by: tarafdarTT <[email protected]>
@jaykru-tt
Copy link
Contributor

I didn't manage to land a fix for this in my PR, but we'll be tackling this as an onboarding exercise for a new teammate in the New Year :)

ayerofieiev-tt added a commit that referenced this issue Dec 17, 2024
### Ticket
#15603

### Problem description
ttnn.pad call throws an error when user attempts to pad a tensor to an
odd number of elements in width dimension

### What's changed
Remove the check, it is outdated since we resolved the underlying
constraint

### Checklist
- [x] [Post commit
CI](https://github.com/tenstorrent/tt-metal/actions/runs/12363708529)
@swimdi swimdi closed this as completed Dec 20, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in PyTorch 2.0 TT-NN Compiler Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

4 participants