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

Clean Up Tensor/Sptensor setitem (and getitem to match) #108

Closed
ntjohnson1 opened this issue May 22, 2023 · 3 comments · Fixed by #150
Closed

Clean Up Tensor/Sptensor setitem (and getitem to match) #108

ntjohnson1 opened this issue May 22, 2023 · 3 comments · Fixed by #150
Assignees
Labels
bug Something isn't working doing Actively being worked on

Comments

@ntjohnson1
Copy link
Collaborator

This is an extension from the long discussion here but the short summary is:
This is the example from MATLAB #92 (comment) with this being the example from Danny with additional elaboration on expected behavior #92 (comment). Will follow up on more details about the expected python interface to confirm. I think the pieces we have are fairly close already so this shouldn't impact correctness of existing algorithms but rather to confirm this as a toolbox acts as expected.

@ntjohnson1 ntjohnson1 self-assigned this May 22, 2023
@ntjohnson1 ntjohnson1 added the bug Something isn't working label May 22, 2023
@ntjohnson1 ntjohnson1 added the doing Actively being worked on label May 28, 2023
@ntjohnson1
Copy link
Collaborator Author

Closed with #116

@dmdunla
Copy link
Collaborator

dmdunla commented Jun 13, 2023

Re-opening issue, as the behavior described in #92 (comment) seems to work for __setitem__ but not for __getitem__. Here are the cases for tensor that MATLAB supports (with examples illustrating my understanding of what the corresponding Python call should look like for a 4-way tensor). sptensor may need to be addressed as well, but this is a start for tensor.

Intended behavior (to align with MATLAB):
1a. Single subscript -> scalar: X[0,0,0,0]
1b. Slice -> tensor: X[0,0,0,:], X[:,0,0,:], or X[0:2,[1,3],0,:]
2a. Array (p x N) of subscripts -> array (p x 1): X[np.array([[0,0,0,0],[2,3,1,0]])]
2.b Array (p x 1) of linear indices -> array (p x 1): X[np.array([[0,5,6,22]]).T]

Current behavior:
1a. Working as in MATLAB.
1b. Working as in MATLAB.
2a. Does not seem to be working. Restricted to 2 x N array input and returns a slice not (p x 1) array.
2b. Does not seem to be working. Does not accept array input (only list) and return a 1-D array (which may be OK since we are using numpy, but this breaks from MATLAB).

Steps to reproduce error:

MATLAB:

>> X = tensor(1:24,[3 4 2 1])
X is a tensor of size 3 x 4 x 2 x 1
	X(:,:,1,1) = 
	     1     4     7    10
	     2     5     8    11
	     3     6     9    12
	X(:,:,2,1) = 
	    13    16    19    22
	    14    17    20    23
	    15    18    21    24

>> % Case 1a
>> X(1,1,1,1) 
ans =
     1

>> % Case 1b
>> X(1,1,1,:) 
ans is a tensor of size 1
	ans(:) = 
	     1
>> X(:,1,1,:)
ans is a tensor of size 3 x 1
	ans(:,:) = 
	     1
	     2
	     3
>> X(1:2,[2 4],1,:)
ans is a tensor of size 2 x 2 x 1
	ans(:,:,1) = 
	     4    10
	     5    11
>> X(1:2,[2 4],1,1)
ans is a tensor of size 2 x 2
	ans(:,:) = 
	     4    10
	     5    11

>> % Case 2a
>> S = [1,1,1,1;3,4,2,1]
S =
     1     1     1     1
     3     4     2     1
>> X(S) 
ans =
     1
    24
>> S = [1,1,1,1;2,2,2,1;3,4,2,1]
S =
     1     1     1     1
     2     2     2     1
     3     4     2     1
>> X(S) 
ans =
     1
    17
    24

>> % Case 2b
>> S = [1,6,7,23]'
S =
     1
     6
     7
    23
>> X(S) 
ans =
     1
     6
     7
    23

pyttb:

>>> import pyttb as ttb
>>> import numpy as np

>>> X = ttb.tensor.from_data(np.arange(1,25),(3,4,2,1))
>>> X[:,:,0,0] # printing like MATLAB
tensor of shape 3 x 4
data[:, :] =
[[ 1  4  7 10]
 [ 2  5  8 11]
 [ 3  6  9 12]]
>>> X[:,:,1,0]  # printing like MATLAB
tensor of shape 3 x 4
data[:, :] =
[[13 16 19 22]
 [14 17 20 23]
 [15 18 21 24]]

>>> # Case 1a
>>> X[0,0,0,0]
1

>>> # Case 1b
>>> X[0,0,0,:]
data[:] =
[1]
>>> X[:,0,0,:]
tensor of shape 3 x 1
[[1]
 [2]
 [3]]
>>> X[0:2,[1,3],0,:]
tensor of shape 2 x 2 x 1
data[0, :, :] =
[[ 4]
 [10]]
data[1, :, :] =
[[ 5]
 [11]]
>>> X[0:2,[1,3],0,0]
tensor of shape 2 x 2
data[:, :] =
[[ 4 10]
 [ 5 11]]

>>> # Case 2a
>>> S = [[0,0,0,0],[2,3,1,0]] # list of subscripts lists (should not be allowed)
>>> X[S] # unexpected behavior
array([[1, 1, 1, 1],
       [3, 4, 2, 1]])
>>> S = np.array([[0,0,0,0],[2,3,1,0]]) # array of 2 x N subscripts (should produce array of length 2)
>>> X[S]
array([[ 7, 19],
       [10, 22],
       [ 4, 16],
       [ 1, 13]])
>>> S = np.array([[0,0,0,0],[1,1,1,0],[2,3,1,0]]) # array of 3 x N subscripts (should produce array of length 3)
>>> X[S]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\dmdunla\dev\github.com\pyttb\pyttb\tensor.py", line 1524, in __getitem__
    a = np.squeeze(self.data[tuple(subs)])
IndexError: index 2 is out of bounds for axis 2 with size 2

>>> # Case 2b
>>> S = [0,5,6,22] # list of 4 linear indices (should not be allowed)
>>> X[S]
array([ 1,  6,  7, 23])
>>> S = np.array([[0,5,6,22]]).T # array of 4 x 1 linear indices (shoulf produce array of length 4)
>>> X[S]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\dmdunla\dev\github.com\pyttb\pyttb\tensor.py", line 1524, in __getitem__
    a = np.squeeze(self.data[tuple(subs)])
IndexError: index 5 is out of bounds for axis 1 with size 4

@dmdunla dmdunla reopened this Jun 13, 2023
@dmdunla dmdunla removed the doing Actively being worked on label Jun 13, 2023
@ntjohnson1 ntjohnson1 added the doing Actively being worked on label Jun 17, 2023
@ntjohnson1 ntjohnson1 linked a pull request Jun 17, 2023 that will close this issue
@ntjohnson1
Copy link
Collaborator Author

My slight preference would be to open a new ticket rather than re-opening an old one unless its the EXACT same underlying bug. Our indexing is fairly complicated, so I expect we will continue to have bugs/extensions around it. The PR I just opened ONLY attempts to address the tensor indexing pointed out above. So if that PR resolves I'd like to close the ticket and when the sptensor tutorial is prepared and we figure out bugs there we can open a new ticket. Otherwise there is some ambiguity as to when we could close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working doing Actively being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants