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 chunk Tensor Method #970

Closed
2 tasks done
dcvz opened this issue Nov 19, 2023 · 8 comments
Closed
2 tasks done

Add chunk Tensor Method #970

dcvz opened this issue Nov 19, 2023 · 8 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@dcvz
Copy link
Contributor

dcvz commented Nov 19, 2023

Feature description

Add a chunk method on tensor to faciliate splitting a tensor into a given number of chunks

Feature motivation

Its a fairly common operation supported by other ML frameworks.

Actions

@antimora antimora added the good first issue Good for newcomers label Nov 19, 2023
@Kelvinyu1117
Copy link
Contributor

I would like to work on this issue, can you assign it to me?

@dcvz
Copy link
Contributor Author

dcvz commented Nov 20, 2023

I would like to work on this issue, can you assign it to me?

Nice! Let me know if you need any guidance. You can also find me in the Discord as @dcvz.

@antimora
Copy link
Collaborator

I would like to work on this issue, can you assign it to me?

Done

@Kelvinyu1117
Copy link
Contributor

Kelvinyu1117 commented Nov 21, 2023

I just had a brief look at the source code, the file I should modify is src/tensor/api/base.rs right? And the interface is as follows:

pub fn chunk<const D2: usize>(
    Tensor<B, D2, K>& tensor, 
    chunks: usize, 
    dim: usize = 0) -> Vec<Tensor<B, D2, K>>
{
}

@dcvz
Copy link
Contributor Author

dcvz commented Nov 21, 2023

I just had a brief look at the source code, the file I should modify is src/tensor/api/base.rs right? And the interface is as follows:

Correct! I think interface can likely be the following, if you do the chunking in base space:

pub fn chunk(self, chunks: usize, dim: usize) -> Vec<Self>

Although given that some of the backends already have their own implementations, it might be better to do this backend specific instead of general (depends on what optimizations they have). What do you say @nathanielsimard ?

@Kelvinyu1117
Copy link
Contributor

I think leveraging the implementation of the backend is a good idea!

@dcvz dcvz mentioned this issue Nov 24, 2023
2 tasks
@antimora
Copy link
Collaborator

@dcvz can this issue be closed as the main PR is merged?

@Kelvinyu1117
Copy link
Contributor

I'm working on the backend implementation, we can use this issue or open another issue as a tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants