Skip to content

TENSOR: Indexing is complicated. #150

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

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

ntjohnson1
Copy link
Collaborator

Our indexing always makes my head hurt because of the variety of paths through the logic but I think we are getting simpler/more robust. Adding this as a single commit so @DeepBlockDeepak can cherry-pick if desired to test if this unblocks the tensor tutorial work. Otherwise we can merge and iterate. See commit for the couple of changes I made. I updated our tests to cover the failure modes you described but let me know if you think I need to add something additional.

Interface Note: MATLAB uses a lot of column vectors to resolve ambiguities but those are generally less common in python since numpy defaults to a more generic (length,) array. My preference is to take a list or effective row vector for linear indices and force people trying to use subscripts to always to provide a matrix np.array([[0,1,2]]) even if the nesting isn't generically necessary.

For the exact motivation, here is the behavior off the branch which I believe aligns mostly with expectations.

>>> import pyttb as ttb
>>> import numpy as np
>>> X = ttb.tensor.from_data(np.arange(1,25),(3,4,2,1))
>>> S = [[0,0,0,0],[2,3,1,0]]
>>> X[S]
Traceback (most recent call last):
  File "<path>\Anaconda3\envs\pyttb\lib\site-packages\IPython\core\interactiveshell.py", line 3505, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-6-7b0c9de99a5d>", line 1, in <module>
    X[S]
  File "<path>\pyttb\pyttb\tensor.py", line 1573, in __getitem__
    assert False, "Invalid use of tensor getitem"
AssertionError: Invalid use of tensor getitem
>>> S = np.array([[0,0,0,0],[1,1,1,0],[2,3,1,0]])
>>> X[S]
array([ 1, 17, 24])
>>> S = [0,5,6,22]
>>> X[S]
array([ 1,  6,  7, 23])
>>> X[np.array(S)]
array([ 1,  6,  7, 23])

* Rework logic on getitem so it is a little simpler to follow
* Add tests to cover cases from issue
* Un-transpose things, which is good for sptensor usages
* Fix setitem bug for multi-subscript inserts on non-symetric tensors
* Update doc strings for tensor seitem/getitem
@ntjohnson1 ntjohnson1 linked an issue Jun 17, 2023 that may be closed by this pull request
@ntjohnson1 ntjohnson1 marked this pull request as ready for review June 17, 2023 21:28
@ntjohnson1 ntjohnson1 requested a review from dmdunla June 17, 2023 21:28
@dmdunla dmdunla merged commit 0dd4480 into sandialabs:main Jun 29, 2023
@ntjohnson1 ntjohnson1 deleted the get_item_inconsistencies branch June 29, 2023 16:13
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.

Clean Up Tensor/Sptensor setitem (and getitem to match)
2 participants