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.

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

Add support for rank-n tensors to tilize and untilize #15520

Merged
merged 8 commits into from
Nov 28, 2024

Conversation

jaykru-tt
Copy link
Contributor

@jaykru-tt jaykru-tt commented Nov 27, 2024

Ticket

#15165

What's changed

  • Adds support for rank-n tensors to the tilize and untilize ops. In particular this introduces support for 5D tensors.
  • We support rank-n tensors by first squeezing to a supported rank <= 4, then performing the tilize/untilize, then unsqueezing back to the original shape/rank.
  • Adds tests for a handful of 5D cases to both the untilize and tilize unit test suites.

Checklist

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

@jaykru-tt jaykru-tt force-pushed the jkruer/ndiml_untilize_tilize branch from 174d1ad to 1b2787f Compare November 28, 2024 05:29
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

auto unsqueezed_tensor = ttnn::reshape(output, *original_shape);
return unsqueezed_tensor;
},
.operation = std::move(base_untilize)});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ performance-unnecessary-value-param ⚠️
parameter base_untilize is passed by value and only copied once; consider moving it to avoid unnecessary copies

Suggested change
.operation = std::move(base_untilize)});
.operation = std::move(base_untilize)});

auto unsqueezed_tensor = ttnn::reshape(output, *original_shape);
return unsqueezed_tensor;
},
.operation = std::move(base_tilize)});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ performance-unnecessary-value-param ⚠️
parameter base_tilize is passed by value and only copied once; consider moving it to avoid unnecessary copies

Suggested change
.operation = std::move(base_tilize)});
.operation = std::move(base_tilize)});

@jaykru-tt
Copy link
Contributor Author

I messed up a few of the tilize tests with an unnecessary refactor. Re-running post-commit to be safe then should be able to merge.

@jaykru-tt jaykru-tt force-pushed the jkruer/ndiml_untilize_tilize branch from d613360 to 3c434b9 Compare November 28, 2024 05:51
@jaykru-tt jaykru-tt merged commit efc6f70 into main Nov 28, 2024
9 checks passed
@jaykru-tt jaykru-tt deleted the jkruer/ndiml_untilize_tilize branch November 28, 2024 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants