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

Generalize nlp create head decode #12663

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Generalize nlp create head decode #12663

merged 3 commits into from
Sep 16, 2024

Conversation

caixunshiren
Copy link
Contributor

Ticket

#12393

The following functionality and bug fixes are added:

  • bug 1: when passing in q_heads=32, the output shape is padded to 64 and has to be sliced. If we directly change the output shape to 32 in create_output_tensor function, the output has bad pcc. This is a trace of potential implementation flaws

In addition, the following optimzation should be made to the op:

  • allow interleaved input
  • allow input to be width sharded by arbitrary shape on core grid
  • should output tensor with padded shape if the number head is less than a full tile

@caixunshiren caixunshiren self-assigned this Sep 13, 2024
@caixunshiren
Copy link
Contributor Author

all post commit: https://github.com/tenstorrent/tt-metal/actions/runs/10853903930

@caixunshiren
Copy link
Contributor Author

FYI @sraizada-tt

TT-BrianLiu
TT-BrianLiu approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@TT-BrianLiu TT-BrianLiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment

...eads_decode/device/kernels/reader_interleaved_tm_tile_layout_nlp_create_qkv_heads_decode.cpp Outdated Show resolved Hide resolved
@caixunshiren caixunshiren force-pushed the generalize-nlp-create-head-decode branch from 1ef1b0b to 8fb7a90 Compare September 16, 2024 15:45
@caixunshiren caixunshiren force-pushed the generalize-nlp-create-head-decode branch from 8fb7a90 to e7937dc Compare September 16, 2024 15:47
@caixunshiren caixunshiren merged commit 50c8ae0 into main Sep 16, 2024
6 checks passed
@caixunshiren caixunshiren deleted the generalize-nlp-create-head-decode branch September 16, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TT-BrianLiu TT-BrianLiu TT-BrianLiu approved these changes

@eyonland eyonland Awaiting requested review from eyonland

@patrickroberts patrickroberts Awaiting requested review from patrickroberts

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

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

@xanderchin xanderchin Awaiting requested review from xanderchin

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

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

Assignees

@caixunshiren caixunshiren

Labels
P1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@caixunshiren @TT-BrianLiu