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

[jit] allow slicing multiple dimensions with indices #45239

Closed
wants to merge 5 commits into from

Conversation

Lilyjjo
Copy link
Contributor

@Lilyjjo Lilyjjo commented Sep 23, 2020

Stack from ghstack:

Differential Revision: D23886919

Lilyjjo added a commit that referenced this pull request Sep 23, 2020
ghstack-source-id: 9fd59f36c81182b82a2684cd2b53a928336579b5
Pull Request resolved: #45239
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Sep 23, 2020
@Lilyjjo Lilyjjo linked an issue Sep 23, 2020 that may be closed by this pull request
x = x[[0, 1], :, [1]]
return x

test_func(test_index_slice1, (a,))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use self.checkScript for same functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't use that because I wasn't sure if it was comparing the outputs of the scripted vs non-scripted modules. I double checked on the self.checkScript code and I see that it actually does. I'll change it to use that instead :)

test/jit/test_list_dict.py Show resolved Hide resolved
@dr-ci
Copy link

dr-ci bot commented Sep 23, 2020

💊 CI failures summary and remediations

As of commit 0c84b75 (more details on the Dr. CI page):


Commit 0c84b75 was recently pushed. Waiting for builds...


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 13 times.

Lilyjjo added a commit that referenced this pull request Oct 1, 2020
Currently investigating issue where indexing with an empty list
in eager returns a tensor with a zero-sized dim in the empty
list spot *OTOH* the scripted module will throw this error:
" RuntimeError: The following operation failed in the TorchScript interpreter.
Traceback of TorchScript (most recent call last):
RuntimeError: Input must be of ints, floats, or bools, got Tensor
Empty lists default to List[Tensor]. Add a variable annotation to the
assignment to create an empty list of another type
(torch.jit.annotate(List[T, []]) where T is the type of elements in the
list for Python 2) "

ghstack-source-id: 5f830201f21d626dadef63a9c1677cd90e5078d3
Pull Request resolved: #45239
@Lilyjjo Lilyjjo changed the title [jit] allow slicing multiple dimensions with indicies [WIP] [jit] allow slicing multiple dimensions with indices Oct 1, 2020
Currently investigating issue where indexing with an empty list in eager returns a tensor with a zero-sized dim in the empty list spot *OTOH* the scripted module will throw this error:
" RuntimeError: The following operation failed in the TorchScript interpreter.
    Traceback of TorchScript (most recent call last):
    RuntimeError: Input must be of ints, floats, or bools, got Tensor
    Empty lists default to List[Tensor]. Add a variable annotation to the
    assignment to create an empty list of another type
    (torch.jit.annotate(List[T, []]) where T is the type of elements in the
    list for Python 2) "

Differential Revision: [D23886919](https://our.internmc.facebook.com/intern/diff/D23886919)

[ghstack-poisoned]
Currently investigating issue where indexing with an empty list in eager returns a tensor with a zero-sized dim in the empty list spot *OTOH* the scripted module will throw this error:
" RuntimeError: The following operation failed in the TorchScript interpreter.
    Traceback of TorchScript (most recent call last):
    RuntimeError: Input must be of ints, floats, or bools, got Tensor
    Empty lists default to List[Tensor]. Add a variable annotation to the
    assignment to create an empty list of another type
    (torch.jit.annotate(List[T, []]) where T is the type of elements in the
    list for Python 2) "

Differential Revision: [D23886919](https://our.internmc.facebook.com/intern/diff/D23886919)

[ghstack-poisoned]
Lilyjjo added a commit that referenced this pull request Oct 5, 2020
Currently investigating issue where indexing with an empty list
in eager returns a tensor with a zero-sized dim in the empty
list spot *OTOH* the scripted module will throw this error:
" RuntimeError: The following operation failed in the TorchScript interpreter.
Traceback of TorchScript (most recent call last):
RuntimeError: Input must be of ints, floats, or bools, got Tensor
Empty lists default to List[Tensor]. Add a variable annotation to the
assignment to create an empty list of another type
(torch.jit.annotate(List[T, []]) where T is the type of elements in the
list for Python 2) "

ghstack-source-id: 10c0d1a8d164e9dfe8b1ec6a008782d98388a309
Pull Request resolved: #45239
@Lilyjjo Lilyjjo requested a review from gmagogsfm October 5, 2020 18:21
x = x[[], :, :]
return x

self.checkScript(test_index_slice1, (a,))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move check right next to its function under test, ideally all of them should become individual small testcases

with self.assertRaisesRegex(RuntimeError, "index 4 is out of bounds for dimension 0 with size 3"):
self.checkScript(test_index_slice4, (a,))

with self.assertRaises(RuntimeError):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this test case is needed, please make sure empty sequence with torch.jit.annotate works.

Lilyjjo added a commit that referenced this pull request Oct 5, 2020
Currently investigating issue where indexing with an empty list
in eager returns a tensor with a zero-sized dim in the empty
list spot *OTOH* the scripted module will throw this error:
" RuntimeError: The following operation failed in the TorchScript interpreter.
Traceback of TorchScript (most recent call last):
RuntimeError: Input must be of ints, floats, or bools, got Tensor
Empty lists default to List[Tensor]. Add a variable annotation to the
assignment to create an empty list of another type
(torch.jit.annotate(List[T, []]) where T is the type of elements in the
list for Python 2) "

ghstack-source-id: 970d1d69827fbbb7e62aa5ab3d071cc399b8997b
Pull Request resolved: #45239
@Lilyjjo Lilyjjo requested a review from gmagogsfm October 5, 2020 19:19
@Lilyjjo Lilyjjo changed the title [WIP] [jit] allow slicing multiple dimensions with indices [jit] allow slicing multiple dimensions with indices Oct 5, 2020
@facebook-github-bot
Copy link
Contributor

@Lilyjjo merged this pull request in 9a668f9.

@facebook-github-bot facebook-github-bot deleted the gh/Lilyjjo/7/head branch October 9, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[jit] Support slicing multiple dimensions with sequences
4 participants