#5044: Add optional tensor output for composite ops #8595
+140
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ruthreshx
requested review from
TT-BrianLiu,
tt-aho and
mywoodstock
as code owners
ruthreshx
requested review from
umadevimcw,
eyonland,
bharane-ab,
VirdhatchaniKN,
Aswinmcw,
mcw-anasuya,
mouliraj-mcw and
KalaivaniMCW
dongjin-na
reviewed
May 21, 2024
|
|
||
| if(output_tensor.has_value()) | ||
| { | ||
| output_tensor = where(eqz(input_c, output_mem_config), |
There was a problem hiding this comment.
(modified) The result of tt_eltwise_addcdiv_with_output does not seem to be stored in the output_tensor object passed by the user, but instead in a new tt_result object. What do you think?"
There was a problem hiding this comment.
I don't think so, if the output tensor is passed by the user, the result should be stored in output tensor else we may create a new tensor.
There was a problem hiding this comment.
It’s not stored but replaced. If it’s working correctly, t2 and output_tensor should be the same.
> /home/ubuntu/tt-metal/tests/tt_eager/python_api_testing/sweep_tests/tt_lib_ops.py(589)eltwise_addcdiv_with_output()
-> return tt2torch_tensor(t3)
(Pdb) l
584 t2 = setup_tt_tensor(z, device, layout[2], input_mem_config[2], dtype[2])
585 output_tensor = setup_tt_tensor(z, device, layout[2], input_mem_config[2], dtype[2])
586 t3 = ttl.tensor.addcdiv(t0, t1, t2, scalar, output_mem_config, output_tensor)
587 breakpoint()
588
589 -> return tt2torch_tensor(t3)
590
591
592 @setup_host_and_device
593 def unary_div_bw(
594 x, # grad_tensor
(Pdb) output_tensor
ttnn.Tensor([[[[-41.00000, -88.50000, ..., -42.50000, -28.62500],
[26.00000, -14.81250, ..., -53.50000, 61.75000],
...,
[-44.50000, 39.00000, ..., -23.00000, 32.00000],
[76.50000, 69.00000, ..., -7.40625, 70.50000]]]], shape=Shape([1, 1, 32, 32]), dtype=DataType::BFLOAT16, layout=Layout::TILE)
(Pdb) t3
ttnn.Tensor([[[[-28.50000, -39.25000, ..., -28.00000, 28.25000],
[71.00000, 56.25000, ..., 3.60938, 11.68750],
...,
[17.62500, -22.00000, ..., 10.25000, -10.31250],
[-2.95312, -17.62500, ..., -13.50000, 13.81250]]]], shape=Shape([1, 1, 32, 32]), dtype=DataType::BFLOAT16, layout=Layout::TILE)
There was a problem hiding this comment.
At the very least, the last op in the composite op should take the output tensor parameter and store the result in it. CC: @eyonland
TT-BrianLiu
approved these changes
May 21, 2024
ruthreshx
force-pushed
the
ruthresh/composite_optional
branch
from
c39df22
to
1b30e76
Compare
ruthreshx
force-pushed
the
ruthresh/composite_optional
branch
from
1b30e76
to
35c7c46
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the optional tensor support for addcdiv for an initial example.
Once it is been approved, further implementing the same for all the composite structures.
Primary issue : #5044