-
Notifications
You must be signed in to change notification settings - Fork 6
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
Convert aten.ceil
to ttnn.ceil
#198
Conversation
@jdh8 , lets fire a ticket and lets overcome in compiler by squeezing the dimension? |
bc4e92b
to
1afebf9
Compare
Unfortunately, the workaround by squeezing out the extraneous dimension for 1-D tensors (e6e9ef8) still leaves errors. It cannot resolve tenstorrent/tt-metal#12671.
|
I suspect the problem is the tensor returned by
|
56af042
to
1845ac2
Compare
1845ac2
to
c64f168
Compare
c64f168
to
a62fc8e
Compare
My test results:
|
a5c1d24
to
408f6ba
Compare
All the failing ops are pytest tests/autogen_op/U*-train/*aten_max_pool2d_with_indices_backward_default.py |
d2de299
to
2c17e8d
Compare
204dfe0
to
4c07949
Compare
Tests are running at https://github.com/tenstorrent/pytorch2.0_ttnn/actions/runs/12038678310 |
return unsqueeze_to_2d(TTNN_POINTWISE_UNARY_OPS[node.target]) | ||
|
||
if node.target == torch.ops.aten.round.default: | ||
return unsqueeze_to_2d(ttnn.round, (args[0],), {"decimals": 0}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must update ttnn op binding to default the argument to 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be great! I once tried at tenstorrent/tt-metal#13851 but in vain.
return result | ||
|
||
if node.target in TTNN_POINTWISE_UNARY_OPS: | ||
return unsqueeze_to_2d(TTNN_POINTWISE_UNARY_OPS[node.target]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do we unsqueeze here? Why only for unary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for inspiring me with a simpler algorithm. Reshape the result back when ndims < 2
.
I cannot locally reproduce the errors found in CI.
However, ResNet raises these errors only locally.
|
@kevinwuTT if you can suggest anything to enable debugging of this case |
fe9f57c
to
59c0fb2
Compare
The results are inconclusive: - `cos` passes - `sqrt` and `floor` fail
59c0fb2
to
09535cd
Compare
Tests finally pass! |
Ticket
Problem description
Convert
aten.ceil
tottnn.ceil
and probably other rounding opsCurrently, 1-D cases fail because
ttnn.ceil
produces 2-D results. For instance,ttnn.ceil
takes a (1066,) tensor but produces a (1, 1066) tensor.I doubt if this happens to other elementwise ops, too.
What's changed
aten.ceil
tottnn.ceil
aten.ceil
aten.round
tottnn.round
aten.round
aten.trunc
tottnn.trunc
aten.trunc