[dynamo] Add itertools.repeat via polyfill
#110953
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/110953
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 144ae40 with merge base de3ae93 ( UNSTABLE - The following job failed but was likely due to flakiness present on trunk and has been marked as unstable:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
Didn't find following labels among repository labels: release note: dynamo |
|
@pytorchbot label "release notes: dynamo" |
|
|
||
| if len(args) < 2: | ||
| # We cannot risk infinite generator being consumed to exhaustion by dynamo | ||
| # (i.e. infinite loop) |
There was a problem hiding this comment.
How come? Shouldn't this be triggering the generator protocol?
There was a problem hiding this comment.
Hmm, for some reason, it hangs.
I implemented it as:
def repeat(item, count):
if count is None:
while True:
yield item
# Eager
>>> for i, j in enumerate(repeat(3, None)):
... print(i, j)
... if i > 5:
... break
...
0 3
1 3
2 3
3 3
4 3
5 3
6 3
# dynamo
# *hangs*There was a problem hiding this comment.
The reason is that this does not get wrapped into a generator, but rather a ListIteratorVariable for some reason.
There was a problem hiding this comment.
In particular, InliningGeneratorTranslator simply yields the entire generator into a list, which is then returned as a ListIteratorVariable:
pytorch/torch/_dynamo/symbolic_convert.py
Line 2518 in 8bc04f4
There was a problem hiding this comment.
I will implement general infinite iterators (count, repeat, cycle) in a follow up PR.
These are not preferred as they return an opaque variabletracker. In particular, one cannot do enumerate(repeat(1)). repeat(1, 10) benefits from the integration enjoyed by ListVariableIterator
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 mandatory check(s) failed. The first few are: Dig deeper by viewing the failures on hud |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…110967) Fixes https://github.com/pytorch/pytorch/pull/110953/files#r1352868935 Depends on: #110953 Why not use these for `repeat(item, count)`: > These are not preferred as they return an opaque VariableTracker. In particular, one cannot do `enumerate(repeat(1))`. `repeat(1, 10)` benefits from the integration enjoyed by `ListVariableIterator` Follow ups: - [ ] make listiterator an IteratorVariable, define iterator integrations on base IteratorVariable where unspecialized #110967 (comment) - Please make a new issue for this - [ ] explore integrating cpython itertools test suite #110967 (comment) - [ ] Use something other than `StopIteration` to handle iterator termination #110967 (comment) - [ ] Add test case for consuming iterator simultaneously from two code points https://github.com/pytorch/pytorch/pull/110967/files#r1358325511 Pull Request resolved: #110967 Approved by: https://github.com/ezyang
…ytorch#110967) Fixes https://github.com/pytorch/pytorch/pull/110953/files#r1352868935 Depends on: pytorch#110953 Why not use these for `repeat(item, count)`: > These are not preferred as they return an opaque VariableTracker. In particular, one cannot do `enumerate(repeat(1))`. `repeat(1, 10)` benefits from the integration enjoyed by `ListVariableIterator` Follow ups: - [ ] make listiterator an IteratorVariable, define iterator integrations on base IteratorVariable where unspecialized pytorch#110967 (comment) - Please make a new issue for this - [ ] explore integrating cpython itertools test suite pytorch#110967 (comment) - [ ] Use something other than `StopIteration` to handle iterator termination pytorch#110967 (comment) - [ ] Add test case for consuming iterator simultaneously from two code points https://github.com/pytorch/pytorch/pull/110967/files#r1358325511 Pull Request resolved: pytorch#110967 Approved by: https://github.com/ezyang
…ytorch#110967) Fixes https://github.com/pytorch/pytorch/pull/110953/files#r1352868935 Depends on: pytorch#110953 Why not use these for `repeat(item, count)`: > These are not preferred as they return an opaque VariableTracker. In particular, one cannot do `enumerate(repeat(1))`. `repeat(1, 10)` benefits from the integration enjoyed by `ListVariableIterator` Follow ups: - [ ] make listiterator an IteratorVariable, define iterator integrations on base IteratorVariable where unspecialized pytorch#110967 (comment) - Please make a new issue for this - [ ] explore integrating cpython itertools test suite pytorch#110967 (comment) - [ ] Use something other than `StopIteration` to handle iterator termination pytorch#110967 (comment) - [ ] Add test case for consuming iterator simultaneously from two code points https://github.com/pytorch/pytorch/pull/110967/files#r1358325511 Pull Request resolved: pytorch#110967 Approved by: https://github.com/ezyang
4 participants
Fixes #110286
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @chenyang78 @aakhundov @kadeng @ezyang