Skip to content

Conversation

@wanchaol
Copy link
Collaborator

@wanchaol wanchaol commented Jun 19, 2019

Stack from ghstack:

Differential Revision: D15948547

[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
Value* cur_elem = nullptr;
if (val_type->cast<ListType>()) {
cur_elem = g.insert(aten::select, {val, i}, {}, loc);
} else if (val_type->cast<StringType>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to standardize a builtin like "getelem" to be used here rather than have a big if branch that emits different code, similar to our other magic methods. Furthermore, we should make serialization emit this as foo[i] so that we do not end up getting committed to preserving these operator names.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree with this, will make a separate PR to define it. We will need to follow up on design the deprecation strategy of those prim operators that should be deleted.

wanchaol added 5 commits June 21, 2019 12:09
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
[jit] add for in string support

gh-metadata: pytorch pytorch 21990 gh/wanchaol/24/head
@zou3519 zou3519 deleted the gh/wanchaol/24/head branch June 24, 2019 03:52
@facebook-github-bot
Copy link
Contributor

@wanchaol merged this pull request in c9344fc.

wanchaol added a commit that referenced this pull request Jun 26, 2019
Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use
wanchaol added a commit that referenced this pull request Jun 26, 2019
Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jun 28, 2019
Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jun 28, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 3, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 3, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 3, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 3, 2019
Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 10, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
wanchaol added a commit that referenced this pull request Jul 10, 2019
[jit] register __getitem__ builtin

Summary:

Follow up of #21990, I am switching index select operations to a standard __getitem__ builtin, rather than bunch of different builtins according to the type, such as prim::DictIndex, prim::ListIndex, etc. This will also aligned with the some other magic methods that we already use

gh-metadata: pytorch pytorch 22276 gh/wanchaol/28/head
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.

6 participants