#0: Port eltwise and some misc ops to use TensorSpec #15471
Conversation
sminakov-tt
requested review from
TT-BrianLiu,
yan-zaretskiy,
ntarafdar,
tt-aho,
yugi957,
sjameelTT,
jaykru-tt,
llongTT,
patrickroberts,
eyonland,
ayerofieiev-tt,
dmakoviichuk-tt and
cfjchu
as code owners
| const auto& tensor_b = tensor_args.input_tensor_b; | ||
| const auto input_shape_b = tensor_b.has_value() ? tensor_b->shape() : ttnn::Shape{1, 1}; | ||
| const auto input_shape_b = tensor_b.has_value() ? tensor_b->logical_shape() : ttnn::SimpleShape{}; |
There was a problem hiding this comment.
Has value what does it mean?
There was a problem hiding this comment.
tensor_b is optional here. I'm unsure what it means for a binary op to have a second argument as optional, but preserving the behavior
There was a problem hiding this comment.
I remember now. This is how float scalar is handled. Think of it like there is a union. It’s or float or tensor. Just expressed in this way
| if (output_tensor.has_value()) { | ||
| return output_tensor.value(); | ||
| } | ||
| auto output_shape = compute_broadcasted_output(input_shape_a, input_shape_b); | ||
|
|
||
| auto program_factory = select_program_factory(operation_attributes, tensor_args); |
There was a problem hiding this comment.
Got a bit surprised that we call this method here. It means we call it at least two times per operation call. Not that it’s a problem but it’s a surprise.
There was a problem hiding this comment.
Yeah. I would expect that select_program_factory should be really quick. It makes sense, at least to some extent, that the output tensor specs depend on the actual factory being used for these input arguments.
| if (tensor_args.output_tensor.has_value()) { | ||
| return *tensor_args.output_tensor; | ||
| } | ||
| return create_device_tensor(compute_output_specs(operation_attributes, tensor_args), tensor_args.input_tensor_a.device()); |
There was a problem hiding this comment.
So interesting. Tensor might have multiple devices today. I see this part of code did not change, but I wonder..
On this level multidevice basically does not exist today if I get it right.
There was a problem hiding this comment.
Yes. Multi-device seems to be problematic overall at the moment. With having single MeshDevice it should work as expected though.
|
@ntarafdar None of the ops did anything before to request a custom padding. |
|
@sminakov-tt awesome. Ya next week I'm going to do the same for data movement ops. I think only transpose uses padding for anything so hoping it should be rather smooth for me as well! |
Ticket
Problem description
We need to migrate all ops to use
compute_output_specswith TensorSpec, instead of oldercompute_output_shapesWhat's changed
Migrated ops to TensorSpec:
Fixes in the infrastructure to support the migration.
Checklist