#14898: pass in pad value to transpose in reduce #17142
Merged
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
bbradelTT
requested review from
ayerofieiev-tt,
dmakoviichuk-tt,
rfurko-tt,
cfjchu,
TT-BrianLiu,
razorback3,
dongjin-na,
asandhupatlaTT and
sjameelTT
as code owners
| @@ -34,6 +34,7 @@ struct Reduce { | |||
| }; | |||
|
|
|||
| // Entry point for pool op, which uses non-standard tensors that cannot be padded. | |||
| [[deprecated]] | |||
There was a problem hiding this comment.
Fyi, you can specify a message if you want
[[deprecated("Use calcSomethingDifferently(int).")]]
ayerofieiev-tt
approved these changes
Jan 27, 2025
asandhupatlaTT
approved these changes
Jan 27, 2025
williamlyTT
pushed a commit
that referenced
this pull request
Jan 30, 2025
### Ticket Link to Github Issue #14898 Subset of previous PR (#16989) that caused a hang in (Single-card) Demo tests and got reverted. Verified that this pipeline passes for this subset of changes: https://github.com/tenstorrent/tt-metal/actions/runs/12992459972 ### Problem description - transpose was filling in non-logical areas with default pad value when called from reduce ### What's changed - pass in an appropriate pad value for transpose to use - also mark a method that should only be used by pool to be deprecated to deter other uses ### Checklist - [x] Post commit CI passes https://github.com/tenstorrent/tt-metal/actions/runs/12992465641 - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (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 - [x] New/Existing tests provide coverage for changes
yieldthought
pushed a commit
that referenced
this pull request
Jan 31, 2025
### Ticket Link to Github Issue #14898 Subset of previous PR (#16989) that caused a hang in (Single-card) Demo tests and got reverted. Verified that this pipeline passes for this subset of changes: https://github.com/tenstorrent/tt-metal/actions/runs/12992459972 ### Problem description - transpose was filling in non-logical areas with default pad value when called from reduce ### What's changed - pass in an appropriate pad value for transpose to use - also mark a method that should only be used by pool to be deprecated to deter other uses ### Checklist - [x] Post commit CI passes https://github.com/tenstorrent/tt-metal/actions/runs/12992465641 - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (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 - [x] New/Existing tests provide coverage for changes
hschoi4448
pushed a commit
that referenced
this pull request
Feb 20, 2025
### Ticket Link to Github Issue #14898 Subset of previous PR (#16989) that caused a hang in (Single-card) Demo tests and got reverted. Verified that this pipeline passes for this subset of changes: https://github.com/tenstorrent/tt-metal/actions/runs/12992459972 ### Problem description - transpose was filling in non-logical areas with default pad value when called from reduce ### What's changed - pass in an appropriate pad value for transpose to use - also mark a method that should only be used by pool to be deprecated to deter other uses ### Checklist - [x] Post commit CI passes https://github.com/tenstorrent/tt-metal/actions/runs/12992465641 - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (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 - [x] New/Existing tests provide coverage for changes
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.
Ticket
Link to Github Issue #14898
Subset of previous PR (#16989) that caused a hang in (Single-card) Demo tests and got reverted. Verified that this pipeline passes for this subset of changes: https://github.com/tenstorrent/tt-metal/actions/runs/12992459972
Problem description
What's changed
Checklist