Skip to content

[JIT] Add support for list()#33818

Closed
eellison wants to merge 9 commits intogh/eellison/59/basefrom
gh/eellison/59/head
Closed

[JIT] Add support for list()#33818
eellison wants to merge 9 commits intogh/eellison/59/basefrom
gh/eellison/59/head

Conversation

@eellison
Copy link
Copy Markdown
Contributor

@eellison eellison commented Feb 26, 2020 •
edited
Loading

Stack from ghstack:

Differential Revision: D20121915

[JIT] Add support for list()
ea65ca4 
[ghstack-poisoned]
@eellison eellison requested a review from apaszke as a code owner February 26, 2020 18:26
This was referenced Feb 26, 2020
Closed
Closed
Closed
Closed
Closed
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Feb 26, 2020
@eellison eellison requested review from driazati and suo February 26, 2020 18:29
@eellison
Copy link
Copy Markdown
Contributor Author

eellison commented Feb 26, 2020

There are a few bound aten::list ops. If we had a backwards compat ast-ast writer we could rewrite torch.list -> list and remove them.

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Feb 26, 2020 •
edited
Loading

💊 CircleCI build failures summary and remediations

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

  • 2/2 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following build failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_xla_linux_xenial_py3_6_clang7_test (1/1)

Step: "Test" (full log | pattern match details)

Mar 05 05:23:18 ERROR: test_accurracy (__main__.TrainMnist) 
Mar 05 05:23:14 2020-03-05 05:23:14.803831: I tensorflow/compiler/xla/service/service.cc:176]   StreamExecutor device (0): Host, Default Version 
Mar 05 05:23:14 2020-03-05 05:23:14.809031: I tensorflow/core/distributed_runtime/rpc/grpc_channel.cc:300] Initialize GrpcChannelCache for job localservice -> {0 -> localhost:40960} 
Mar 05 05:23:14 2020-03-05 05:23:14.809412: I tensorflow/core/distributed_runtime/rpc/grpc_server_lib.cc:390] Started server with target: grpc://localhost:40960 
Mar 05 05:23:14 2020-03-05 05:23:14.969273: W tensorflow/compiler/jit/xla_device.cc:398] XLA_GPU and XLA_CPU devices are deprecated and will be removed in subsequent releases. Instead, use either @tf.function(experimental_compile=True) for must-compile semantics, or run with TF_XLA_FLAGS=--tf_xla_auto_jit=2 for auto-clustering best-effort compilation. 
Mar 05 05:23:17 + echo 'Running MNIST Test' 
Mar 05 05:23:17 + python test/test_train_mnist.py --tidy 
Mar 05 05:23:17 Running MNIST Test 
Mar 05 05:23:18  0it [00:00, ?it/s]E 0it [00:00, ?it/s] 
Mar 05 05:23:18  
Mar 05 05:23:18 ====================================================================== 
Mar 05 05:23:18 ERROR: test_accurracy (__main__.TrainMnist) 
Mar 05 05:23:18 ---------------------------------------------------------------------- 
Mar 05 05:23:18 Traceback (most recent call last): 
Mar 05 05:23:18   File "test/test_train_mnist.py", line 186, in test_accurracy 
Mar 05 05:23:18     self.assertGreaterEqual(train_mnist(), FLAGS.target_accuracy) 
Mar 05 05:23:18   File "test/test_train_mnist.py", line 74, in train_mnist 
Mar 05 05:23:18     transforms.Normalize((0.1307,), (0.3081,))])) 
Mar 05 05:23:18   File "/var/lib/jenkins/.local/lib/python3.6/site-packages/torchvision/datasets/mnist.py", line 70, in __init__ 
Mar 05 05:23:18     self.download() 
Mar 05 05:23:18   File "/var/lib/jenkins/.local/lib/python3.6/site-packages/torchvision/datasets/mnist.py", line 137, in download 
Mar 05 05:23:18     download_and_extract_archive(url, download_root=self.raw_folder, filename=filename, md5=md5)
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.

This comment has been revised 46 times.

eellison pushed a commit that referenced this pull request Feb 26, 2020
[JIT] Add support for list()
9fba3bd 
ghstack-source-id: 6e59150
Pull Request resolved: #33818
driazati
driazati approved these changes Feb 26, 2020
View reviewed changes
Comment thread test/test_jit.py
self.checkModule(M2(), (inp, name))

def test_list_keyword(self):
def foo():
Copy link
Copy Markdown
Contributor

@driazati driazati Feb 26, 2020

Choose a reason for hiding this comment

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

Can you add a check for a bad input as well? e.g. that list(1) says something like Python's TypeError: 'int' object is not iterable

Copy link
Copy Markdown
Contributor Author

@eellison eellison Feb 26, 2020

Choose a reason for hiding this comment

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

there are tests that already do that like test_sum_list_wrong_type

eellison pushed a commit that referenced this pull request Feb 26, 2020
[JIT] Add support for list()
f985bea 
ghstack-source-id: 63e5c12
Pull Request resolved: #33818
eellison pushed a commit that referenced this pull request Feb 27, 2020
[JIT] Add support for list()
dcd7820 
ghstack-source-id: 9ec7000
Pull Request resolved: #33818
eellison pushed a commit that referenced this pull request Feb 28, 2020
[JIT] Add support for list()
4250515 
ghstack-source-id: 8abbbcc
Pull Request resolved: #33818
eellison pushed a commit that referenced this pull request Mar 2, 2020
[JIT] Add support for list()
1a4c3fe 
ghstack-source-id: 065f39e
Pull Request resolved: #33818
eellison pushed a commit that referenced this pull request Mar 5, 2020
[JIT] Add support for list()
024982d 
ghstack-source-id: 058a467
Pull Request resolved: #33818
@facebook-github-bot
Copy link
Copy Markdown
Contributor

facebook-github-bot commented Mar 6, 2020

@eellison merged this pull request in f218842.

@mrshenli
Copy link
Copy Markdown
Contributor

mrshenli commented Mar 6, 2020

pytorch_linux_xenial_cuda10_1_cudnn7_py3_slow_test on master starts to time out on this PR. It actually also fails on this PR but passed on the one below it (#33783). Reverting.

Mar 05 07:04:32 Generating XML reports...
Mar 05 07:04:33 Running test_jit ... [2020-03-05 07:04:33.071318]
Mar 05 07:04:36 
Mar 05 07:04:36 Running tests...
Mar 05 07:04:36 ----------------------------------------------------------------------

Too long with no output (exceeded 1h30m0s): context deadline exceeded

@eellison
Copy link
Copy Markdown
Contributor Author

eellison commented Mar 6, 2020

Are you sure that’s related ?

@facebook-github-bot facebook-github-bot deleted the gh/eellison/59/head branch March 9, 2020 14:16
@eellison eellison mentioned this pull request Apr 7, 2020
Closed
@eellison eellison mentioned this pull request Nov 4, 2020
Closed
facebook-github-bot pushed a commit that referenced this pull request Nov 6, 2020
@facebook-github-bot
[JIT][Reland] add list() support (#42382)
f3ad7b2 
Summary:
Fixes #40869

Resubmit of #33818.

Adds support for `list()` by desugaring  it to a list comprehension.

Last time I landed this it made one of the tests slow, and got unlanded. I think that's bc the previous PR changed the emission of `list()` on a list input or a str input to a list comprehension, which is the more general way of emitting `list()`, but also a little bit slower. I updated this version to emit to the builtin operators for these two case. Hopefully it can land without being reverted this time...

Pull Request resolved: #42382

Reviewed By: navahgar

Differential Revision: D24767674

Pulled By: eellison

fbshipit-source-id: a1aa3d104499226b28f47c3698386d365809c23c
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@facebook-github-bot
[JIT] Add support for list() (pytorch#33818)
3b84ec7 
Summary: Pull Request resolved: pytorch#33818

Test Plan: Imported from OSS

Differential Revision: D20121915

Pulled By: eellison

fbshipit-source-id: c6c4ef444dbf1d4134dccb28c13315e225945b64
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
@facebook-github-bot
[JIT][Reland] add list() support (pytorch#42382)
b846897 
Summary:
Fixes pytorch#40869

Resubmit of pytorch#33818.

Adds support for `list()` by desugaring  it to a list comprehension.

Last time I landed this it made one of the tests slow, and got unlanded. I think that's bc the previous PR changed the emission of `list()` on a list input or a str input to a list comprehension, which is the more general way of emitting `list()`, but also a little bit slower. I updated this version to emit to the builtin operators for these two case. Hopefully it can land without being reverted this time...

Pull Request resolved: pytorch#42382

Reviewed By: navahgar

Differential Revision: D24767674

Pulled By: eellison

fbshipit-source-id: a1aa3d104499226b28f47c3698386d365809c23c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apaszke apaszke Awaiting requested review from apaszke

@suo suo Awaiting requested review from suo

1 more reviewer

@driazati driazati driazati approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@eellison @facebook-github-bot @mrshenli @driazati @mruberry