-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Throw error on 0-length tensor slicing #7775
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
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Hum.... I'm not sure we want to disable this behavior altogether, because in the case where the indexing slice is in the first dimension, it kinda works as is now (returns an empty tensor). A better fix I think would be to check if the slice size is 0, and returns an empty tensor straight away? |
|
For example, I think we should have the same behavior for the moment as in the following: Both prints an empty tensor, and I think this is the expected behavior for now. |
|
I haven't thought through the implications of @fmassa's comment, but it seems consistent with how we have handled 0-sized things in the past: all sizes that would include a 0 if we supported it are collapsed to (0,). |
|
One issue with returning a 0 index tensor is when it's not the last index you specify. For example: |
|
Can't we short-circuit indexing if any of the indexing elements is a slice of size zero? Its quite annoying to have checks everywhere in the code to skip some operations if the result of the operation is an empty tensor, and with the current setup, many things work already. |
|
@pytorchbot retest this please |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@li-roy Don't forget about this PR! |
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
4 similar comments
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
zou3519
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
| # indexing 0-length slice | ||
| self.assertEqual(torch.tensor([]), reference[slice(0)]) | ||
| self.assertEqual(torch.tensor([]), reference[slice(0), 2]) | ||
| self.assertEqual(torch.tensor([]), reference[2, slice(0)]) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@pytorchbot retest this please |
Fixes #7559. Throw an error because we don't support dims of size 0 yet.