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

#17477: Move SmallVector and ShapeBase to re-use within Metal #17669

Merged
merged 4 commits into from
Feb 7, 2025

Conversation

omilyutin-tt
Copy link
Contributor

@omilyutin-tt omilyutin-tt commented Feb 6, 2025

Ticket

#17477

Problem description

TT-distributed requires ND shapes in Metal. Instead of having our own, moving ShapeBase and SmallVector into Metal.

What's changed

  • Moved ShapeBase and SmallVector into Metal. SmallVector really should be part of our "stl" library, so I put it there for now. I used ttsl namespace - my goal is to replace tt::stl as it is shorter and easier to type, also avoids confusion if anyone attempts to use stl::.

Checklist

Comment on lines 36 to 39
// TODO: remove this.
namespace ttnn {
using tt::tt_metal::SmallVector;
using ttsl::SmallVector;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd want to do any renames in follow up PRs. Pending conversation on this one.

@omilyutin-tt
Copy link
Contributor Author

@ayerofieiev-tt @sminakov-tt please let me know your opinion here. My plan is to subclass ShapeBase to represent MeshShape, then implement a set of libraries for multi dimensional container, device coordinates, iterator, range. Ideally also re-use that for Core* libs.

We can also move Shape and Alignment into Metal, but I think we can hold on with that. These are more specific to working with Tensors.

Copy link
Member

@ayerofieiev-tt ayerofieiev-tt left a comment

Choose a reason for hiding this comment

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

SmallVector is fine to move

ShapeBase is ooookay to move
What about Shape?

@omilyutin-tt
Copy link
Contributor Author

What about Shape?

No strong opinion here, but my mental model for Shape is that it is a Tensor shape. I'd want to subclass ShapeBase for MeshShape: methods like volume(), rank(), to_array_4D() that are added by Shape sound like they assume we are talking about tensors.

@omilyutin-tt omilyutin-tt merged commit b552fb8 into main Feb 7, 2025
212 checks passed
@omilyutin-tt omilyutin-tt deleted the omilyutin/lower-shapes branch February 7, 2025 05:48
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.

5 participants