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

[new+change+fix] Expanding Array creation Module #137

Draft
wants to merge 35 commits into
base: pre-0.4
Choose a base branch
from

Conversation

shivasankarka
Copy link
Collaborator

@shivasankarka shivasankarka commented Oct 22, 2024

New

  • To implement all the array creation routines from numpy and expand the NuMojo array creation functionality (Most have been implemented now, few more left to implement).

Changes

  • To make matmul flexible for different shapes of input arrays. Since NuMojo uses a convention of ignoring the single index in shape i.e (3, 2, 1) becomes (3, 2), The matmul needs to be modified to account for this in cases like
A(shape = MxN) X B(shape = N)
  • Change the name of the buffer of the NDArray to _buf.
  • Change the way to get the shape of the array. After change, array.shape returns the shape of array as NDArrayShape.

Fix

  • Remove inout before self for NDArray.__getitem__.

@shivasankarka shivasankarka marked this pull request as draft October 22, 2024 05:53
@forFudan
Copy link
Collaborator

Since NuMojo uses a convention of ignoring the single index in shape i.e (3, 2, 1) becomes (3, 2)

Do we have this feature already in the code? I believe that nm.zeros(Shape(3,2,1)) still generate a array with shape 3x2x1 instead of 3x2.

@shivasankarka
Copy link
Collaborator Author

Do we have this feature already in the code? I believe that nm.zeros(Shape(3,2,1)) still generate a array with shape 3x2x1 instead of 3x2.

Let's say we return a slice (arr[:,:,0:1]) of a 3x4x5 array, the resulting array is returned as 3x4 instead of 3x4x1. Most of the function in NuMojo follow this convention right now.

@forFudan
Copy link
Collaborator

Let's say we return a slice (arr[:,:,0:1]) of a 3x4x5 array, the resulting array is returned as 3x4 instead of 3x4x1. Most of the function in NuMojo follow this convention right now.

This is interesting. 0:1 is somehow interpreted as an integer.

@shivasankarka
Copy link
Collaborator Author

@forFudan , Yes, the slices with those consecutive integers are interpreted that way. Now I am wondering which convention would be best to follow. In the current convention, there's no difference between a Nx1 and 1xN matrix (They are interpreted as a size N vector).

@forFudan
Copy link
Collaborator

forFudan commented Oct 22, 2024

@forFudan , Yes, the slices with those consecutive integers are interpreted that way. Now I am wondering which convention would be best to follow. In the current convention, there's no difference between a Nx1 and 1xN matrix (They are interpreted as a size N vector).

@shivasankarka Each Int decrease the dimension by 1. So slide 0:1 should not. This is the convention of numpy and I found this behavior well defined.

Nx1 and 1xN should be differentiated if ndim is 2.

@forFudan
Copy link
Collaborator

@shivasankarka I have an interesting suggestion. Shall we just rename NDArrayShape to Shape, since there is no conflicting definition? Also, rename NDArray.ndshape as NDArray.shape.

@MadAlex1997
Copy link
Collaborator

@shivasankarka I have an interesting suggestion. Shall we just rename NDArrayShape to Shape, since there is no conflicting definition? Also, rename NDArray.ndshape as NDArray.shape.

ndarray.shape in numpy is a tuple representing the shape. Having an iterable thing bound to either ndarray.shape or ndarray.shape() is more convenient than having to go ndarray.shape.buffer.

@forFudan
Copy link
Collaborator

Having an iterable thing bound to either ndarray.shape or ndarray.shape() is more convenient than having to go ndarray.shape.buffer.

Currently, NDArrayShape is already subscripable, which means we can already do this now:

A.ndshape[0]

So my proposal is to make it something like:

A.shape[0]

@shivasankarka
Copy link
Collaborator Author

@shivasankarka I have an interesting suggestion. Shall we just rename NDArrayShape to Shape, since there is no conflicting definition? Also, rename NDArray.ndshape as NDArray.shape.

I don't see a problem with that change right now. To my knowledge, numpy exposes it as np.ndarray.shape(), so that change should be fine. The only thing that might be confusing (for internal developing and debugging purposes) is having a syntax like arr(shape=Shape(...)) instead of arr(shape=NDArrayShape(...)). The argument shape` will keep repeating a lot.

@forFudan
Copy link
Collaborator

To my knowledge, numpy exposes it as np.ndarray.shape()
@shivasankarka numpy did it via np.ndarray.shape member (not method). We currently do this with NDArray.ndshape and NDArray.shape().

forFudan and others added 3 commits October 24, 2024 14:31
@forFudan forFudan changed the title Expanding Array creation Module [new+change+fix] Expanding Array creation Module Oct 24, 2024
Rename the data buffer of `NDArrayShape` as `_buf`
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.

3 participants