Moving DataPipe buffers from __iter__ to instance (self) #388
Conversation
[ghstack-poisoned]
facebook-github-bot
added
the
CLA Signed
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 Note that the CI is expected to fail until that PR lands [ghstack-poisoned]
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 Note that the CI is expected to fail until that PR lands [ghstack-poisoned]
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775 Note that the CI is expected to fail until that PR goes into nightly [ghstack-poisoned]
| if self.buffer: | ||
| self.buffer.clear() |
There was a problem hiding this comment.
Interesting enough the domain CI test for Vision started to fail for a previous commit of this PR. It is raising "ResourceWarning" as you can see in the link below.
https://github.com/pytorch/data/runs/6498262553?check_suite_focus=true
Perhaps we need clear the buffer when the iterator is exhausted? I am adding this line to see if the issue is resolved. Let me know if you have any other thoughts.
There was a problem hiding this comment.
Just tried and clearing the buffer at the end of __iter__ doesn't help.
@pmeier Do you think this is related to pytest or something that is introduced in this PR?
There was a problem hiding this comment.
For reference, here is the thread where we discussed a similar issue:
pytorch/vision#5801
There was a problem hiding this comment.
I am going to ignore the warning and land this based on the previous investigation.
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775 Note that the CI is expected to fail until that PR goes into nightly [ghstack-poisoned]
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775 Note that the CI is expected to fail until that PR goes into nightly [ghstack-poisoned]
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775 Note that the CI is expected to fail until that PR goes into nightly [ghstack-poisoned]
|
@NivekT has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
This PR impacts `ParagraphAggregator` and `IterKeyZipper` as they previously have their buffer with the iterator rather than the instance. This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775 Note that the CI is expected to fail until that PR goes into nightly Differential Revision: [D36519522](https://our.internmc.facebook.com/intern/diff/D36519522) [ghstack-poisoned]
|
@NivekT has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Stack from ghstack:
This PR impacts
ParagraphAggregatorandIterKeyZipperas they previously have their buffer with the iterator rather than the instance.This should be landed along with pytorch/pytorch#76999 and pytorch/pytorch#77775
Note that the CI is expected to fail until that PR goes into nightly
Differential Revision: D36519522