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.

Sign up for GitHub

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

Jump to bottom

#16143: Inplace support for binary_ng ops with fused activations #16449

Merged
merged 2 commits into from
Jan 25, 2025
Merged

#16143: Inplace support for binary_ng ops with fused activations #16449

merged 2 commits into from
Jan 25, 2025

Conversation

mcw-anasuya
Copy link
Contributor

@mcw-anasuya mcw-anasuya commented Jan 6, 2025 •
edited
Loading

Ticket

#16143
#16153

Problem description

Provide inplace support for binary_ng ops with fused activations with validation checks for input.

What's changed

Added in-place support for 20 ops
Added fused activations on input tensors.
Added validation checks for input tensor a.

Checklist

@mcw-anasuya mcw-anasuya changed the base branch from main to yan-zaretskiy/16153-fused-activations January 6, 2025 10:04
@patrickroberts patrickroberts force-pushed the yan-zaretskiy/16153-fused-activations branch 4 times, most recently from e631a50 to fd2b11f Compare January 7, 2025 21:00
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch 2 times, most recently from 61cce7f to 35a785c Compare January 8, 2025 12:19
@mcw-anasuya mcw-anasuya changed the title Inplace support for binary_ng ops with fused activations #16143: Inplace support for binary_ng ops with fused activations Jan 8, 2025
Base automatically changed from yan-zaretskiy/16153-fused-activations to main January 8, 2025 19:15
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch from 1ee72e7 to 6c3825b Compare January 9, 2025 04:59
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch 2 times, most recently from 0f92910 to 4b1f750 Compare January 11, 2025 00:59
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch 2 times, most recently from 677b7e1 to d56bc24 Compare January 13, 2025 13:37
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch from 3dc3d57 to 023014b Compare January 16, 2025 07:22
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch from 023014b to 7c4395f Compare January 17, 2025 18:01
@patrickroberts patrickroberts mentioned this pull request Jan 17, 2025
Closed
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch 4 times, most recently from b3bf5e1 to 5ce375b Compare January 22, 2025 05:36
@ayerofieiev-tt ayerofieiev-tt mentioned this pull request Jan 23, 2025
Merged
6 tasks
@patrickroberts
Copy link
Contributor

I've fixed the merge conflicts that are currently here and I'm testing that the build compiles and the existing tests pass before I force push. Just wanted to share this here so that nobody duplicates efforts on this since it's already done. @mcw-anasuya @umadevimcw

@patrickroberts
Copy link
Contributor

I've fixed the merge conflicts that are currently here and I'm testing that the build compiles and the existing tests pass before I force push. Just wanted to share this here so that nobody duplicates efforts on this since it's already done. @mcw-anasuya @umadevimcw

Turns out I have the commit order messed up so I would have to redo the rebase since it makes me fix the exact same conflicts I already fixed just to reorder the commits from what I have now, which is here: https://github.com/tenstorrent/tt-metal/tree/proberts/inplace_binary_ng
This shows what the code should look like for this PR but it's not really usable for fixing the merge conflicts that are currently present unfortunately because of what I explained above.

@KalaivaniMCW KalaivaniMCW force-pushed the anasuya/inplace_binary_ng branch from 5ce375b to dc8235b Compare January 25, 2025 12:00
@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch 3 times, most recently from 00c2e07 to 56d0311 Compare January 25, 2025 16:16
@mcw-anasuya
Copy link
Contributor Author

I've fixed the merge conflicts that are currently here and I'm testing that the build compiles and the existing tests pass before I force push. Just wanted to share this here so that nobody duplicates efforts on this since it's already done. @mcw-anasuya @umadevimcw

Turns out I have the commit order messed up so I would have to redo the rebase since it makes me fix the exact same conflicts I already fixed just to reorder the commits from what I have now, which is here: https://github.com/tenstorrent/tt-metal/tree/proberts/inplace_binary_ng This shows what the code should look like for this PR but it's not really usable for fixing the merge conflicts that are currently present unfortunately because of what I explained above.

@patrickroberts I have rebased the branch to main and pushed the changes done here 5be36dc

@mcw-anasuya mcw-anasuya force-pushed the anasuya/inplace_binary_ng branch from 56d0311 to 0425093 Compare January 25, 2025 18:35
@mcw-anasuya mcw-anasuya merged commit ad0b806 into main Jan 25, 2025
8 checks passed
@mcw-anasuya mcw-anasuya deleted the anasuya/inplace_binary_ng branch January 25, 2025 18:53
williamlyTT pushed a commit that referenced this pull request Jan 30, 2025
@mcw-anasuya @patrickroberts @williamlyTT
#16143: Inplace support for binary_ng ops with fused activations (#16449
adb389f 
)

### Ticket
#16143
#16153 

### Problem description
Provide inplace support for binary_ng ops with fused activations with
validation checks for input.

### What's changed
Added in-place support for 18 ops 
Added fused activations on input tensors.
Added validation checks for input tensor a.

### Checklist
- [x] Post commit CI passes
https://github.com/tenstorrent/tt-metal/actions/runs/12966520490 -
PASSED
- [x] Blackhole post-commit tests
https://github.com/tenstorrent/tt-metal/actions/runs/12966965482 -
PASSED
- [x] (Single-card) Tests for new models
https://github.com/tenstorrent/tt-metal/actions/runs/12966971183 - same
as main
- [x] New/Existing tests provide coverage for changes

---------

Co-authored-by: Patrick Roberts <[email protected]>
yieldthought pushed a commit that referenced this pull request Jan 31, 2025
@mcw-anasuya @patrickroberts @yieldthought
#16143: Inplace support for binary_ng ops with fused activations (#16449
d10da78 
)

### Ticket
#16143
#16153 

### Problem description
Provide inplace support for binary_ng ops with fused activations with
validation checks for input.

### What's changed
Added in-place support for 18 ops 
Added fused activations on input tensors.
Added validation checks for input tensor a.

### Checklist
- [x] Post commit CI passes
https://github.com/tenstorrent/tt-metal/actions/runs/12966520490 -
PASSED
- [x] Blackhole post-commit tests
https://github.com/tenstorrent/tt-metal/actions/runs/12966965482 -
PASSED
- [x] (Single-card) Tests for new models
https://github.com/tenstorrent/tt-metal/actions/runs/12966971183 - same
as main
- [x] New/Existing tests provide coverage for changes

---------

Co-authored-by: Patrick Roberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@patrickroberts patrickroberts patrickroberts approved these changes

@umadevimcw umadevimcw umadevimcw approved these changes

@eyonland eyonland eyonland approved these changes

@VirdhatchaniKN VirdhatchaniKN VirdhatchaniKN approved these changes

@KalaivaniMCW KalaivaniMCW KalaivaniMCW approved these changes

@ayerofieiev-tt ayerofieiev-tt ayerofieiev-tt approved these changes

@mouliraj-mcw mouliraj-mcw mouliraj-mcw approved these changes

@Aswinmcw Aswinmcw Awaiting requested review from Aswinmcw

@yan-zaretskiy yan-zaretskiy Awaiting requested review from yan-zaretskiy

@dmakoviichuk-tt dmakoviichuk-tt Awaiting requested review from dmakoviichuk-tt dmakoviichuk-tt is a code owner

@cfjchu cfjchu Awaiting requested review from cfjchu cfjchu is a code owner

@TT-BrianLiu TT-BrianLiu Awaiting requested review from TT-BrianLiu TT-BrianLiu is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mcw-anasuya @patrickroberts @umadevimcw @eyonland @VirdhatchaniKN @KalaivaniMCW @ayerofieiev-tt @mouliraj-mcw