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

fix argmax issues #16479

Closed
wants to merge 3 commits into from
Closed

fix argmax issues #16479

wants to merge 3 commits into from

Conversation

asandhupatlaTT
Copy link
Contributor

@asandhupatlaTT asandhupatlaTT commented Jan 7, 2025 •
edited
Loading

Ticket

Link to Github Issue

Problem description

Wrong pcc & shaoe for argmax output

What's changed

Fixed the shape as of now.

Checklist

@asandhupatlaTT asandhupatlaTT self-assigned this Jan 7, 2025
@asandhupatlaTT asandhupatlaTT linked an issue Jan 7, 2025 that may be closed by this pull request
ttnn.argmax low PCC when dim is not None #11550
Closed
@asandhupatlaTT
fix wrong output shape for argmax
808ae99 
Signed-off-by: Amruth Sandhupatla <[email protected]>
@asandhupatlaTT
Copy link
Contributor Author

asandhupatlaTT commented Jan 20, 2025 •
edited
Loading

Creator of original ticket reduced the scope to just mismatch in dimensions. hence, this PR is ready for review & merging

PCC is bad for my custom test case but fix will be part of another ticket & PR

@asandhupatlaTT asandhupatlaTT marked this pull request as ready for review January 20, 2025 19:29
@asandhupatlaTT
address reviewer comments
f355aad 
Signed-off-by: Amruth Sandhupatla <[email protected]>
@asandhupatlaTT
disable failing test case
d38ff16 
Signed-off-by: Amruth Sandhupatla <[email protected]>
@asandhupatlaTT asandhupatlaTT mentioned this pull request Jan 20, 2025
Open
@asandhupatlaTT
Copy link
Contributor Author

Wrong PCC bug is documented at #16922

bbradelTT
bbradelTT requested changes Jan 21, 2025
View reviewed changes
tests/tt_eager/python_api_testing/sweep_tests/pytests/tt_dnn/test_argmax_int.py
assert status
# FIXME: this code is hacky. Looms like there is wrong with argnax code. if we correct wrong dims,
# we get wrong output. Need to fix this.
# assert status
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to at least understand why the change breaks this test before merging the change in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

more details are captured at #16922

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at this. The dimensions are not set up properly in this PR.

Just removing the dimension of size 1 from the output shape works. I'm doing that in #16989 along with some other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should i cancel this PR @bbradelTT since ur PR covers it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

@asandhupatlaTT asandhupatlaTT deleted the asandhupatla/11550 branch January 24, 2025 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbradelTT bbradelTT bbradelTT requested changes

@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

@rfurko-tt rfurko-tt Awaiting requested review from rfurko-tt rfurko-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

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

@dongjin-na dongjin-na Awaiting requested review from dongjin-na dongjin-na is a code owner

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

Assignees

@asandhupatlaTT asandhupatlaTT

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ttnn.argmax low PCC when dim is not None
2 participants
@asandhupatlaTT @bbradelTT