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

Improve consistency in how we pass around dynamic array of elements #17544

Open
dgomezTT opened this issue Feb 4, 2025 · 1 comment
Open
Assignees
Labels
metal tt-metal issue ttnn

Comments

@dgomezTT
Copy link

dgomezTT commented Feb 4, 2025

vector vs SmallVector vs span: There should be a single way to represent a (dynamic) array of elements. The span is an ideal option as it represents an interface rather than a class, but even std::vector or SmallVector are okay if they are used consistently.

@dgomezTT dgomezTT added the metal tt-metal issue label Feb 4, 2025
@dgomezTT dgomezTT self-assigned this Feb 4, 2025
@dgomezTT dgomezTT changed the title Improve consistency in how we store dynamic array of elements Improve consistency in how we pass around dynamic array of elements Feb 4, 2025
@ayerofieiev-tt
Copy link
Member

ayerofieiev-tt commented Feb 4, 2025

From @svuckovicTT

ttnn/cpp/ttnn/operations/reduction/generic/generic_reductions.hpp

struct Reduce {
    static Tensor invoke(
        const Tensor& input_tensor_arg,
        const std::optional<std::variant<int, ttnn::SmallVector<int>>>& dim_arg = std::nullopt,    // <---- smallvec here
        const bool keepdim = true,
        const std::optional<MemoryConfig>& memory_config_arg = std::nullopt,
        const std::optional<DeviceComputeKernelConfig>& compute_kernel_config = std::nullopt,
        float scalar = 1.0f);
};

vs

ttnn/cpp/ttnn/operations/data_movement/permute/permute.hpp

struct ExecutePermute {
    static ttnn::Tensor invoke(
        uint8_t queue_id,
        const ttnn::Tensor& input_tensor,
        const SmallVector<int64_t>& dims,  // <---
        const std::optional<MemoryConfig>& memory_config,
        const std::optional<float>& pad_value = 0.0f);

    static ttnn::Tensor invoke(
        const ttnn::Tensor& input_tensor,
        const SmallVector<int64_t>& dims,  // <---
        const std::optional<MemoryConfig>& memory_config,
        const std::optional<float>& pad_value = 0.0f);

    static ttnn::Tensor invoke(
        const ttnn::Tensor& input_tensor,
        const SmallVector<int64_t>& dims,  // <--- 
        const std::optional<float>& pad_value = 0.0f);
};

Basically, what we want to unify to Span instead of SmallVector in op invoke methods.
Please keep in mind, span does not maintain ownership over data. But from the interface standpoint it is convenient to accept span as it allows whatever input, whether its array, vector, smallvector or any other compatible container.

There are cases where a fixed size array is used. Such cases are out of scope of this effort

struct ExecuteUpSample {
    static ttnn::Tensor invoke(
        const ttnn::Tensor& input_tensor,
        std::variant<int, tt::tt_metal::Array2D> scale_factor,  // <---
        const std::string& mode = std::string("nearest"),
        const std::optional<MemoryConfig>& output_mem_config = std::nullopt,
        const std::optional<DeviceComputeKernelConfig>& compute_kernel_config = std::nullopt);
};

CC @dgomezTT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metal tt-metal issue ttnn
Projects
None yet
Development

No branches or pull requests

2 participants