[WIP][DataPipe] Add RandomSplitter (with buffer) by NivekT · Pull Request #723 · meta-pytorch/data

NivekT · 2022-08-09T22:24:32Z

Stack from ghstack:

-> [WIP][DataPipe] Add RandomSplitter (with buffer) #723

This PR adds RandomSplitter with an implementation that uses a buffer through demux, thus allowing all child DataPipes to be used simultaneously. This may not work for the memory-bound cases.

TODO:

Decide if we like a buffer-less version better. Or we can add both.
Determines if the API related to randomness needs further extension (we might need to add set_seed)
More tests.

See #712 for related discussion.
See #724 for the version WITHOUT buffer.

[ghstack-poisoned]

ghstack-source-id: 783d4a7 Pull Request resolved: #723

NivekT · 2022-08-09T22:35:32Z

@ejguan @VitalyFedyunin This is WIP, we should discuss this but let me know if there is any initial reaction on how we would like to do random_split.

This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). TODO: * Decide if we like this or the buffer version better. Or we can add both. * Determines if the API related to randomness needs further extension (we might need to add set_seed) * More tests. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). TODO: * Decide if we like this or the buffer version better. Or we can add both. * Determines if the API related to randomness needs further extension (we might need to add set_seed) * More tests. Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

…t buffer)" This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). TODO: * Decide if we like this or the buffer version better. Or we can add both. * Determines if the API related to randomness needs further extension (we might need to add set_seed) * More tests. Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). TODO: * Decide if we like this or the buffer version better. Or we can add both. * Determines if the API related to randomness needs further extension (we might need to add set_seed) * More tests. Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

…fer)" This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. * I also decided against having an option to change seed automatically after each iteration, because there are situations where the first iteration is for `test` and the second iteration is for `valid`. Changing seed will be confusing and causes inconsistency. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. * I also decided against having an option to change seed automatically after each iteration, because there are situations where the first iteration is for `test` and the second iteration is for `valid`. Changing seed will be confusing and causes inconsistency. See #712 for related discussion. See #723 for the version with buffer. [ghstack-poisoned]

NivekT · 2022-08-12T22:25:23Z

Closing this PR for now since we are moving forward with the buffer-less version. We can re-visit this if there is a need for it.

…fer)" This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. * I also decided against having an option to change seed automatically after each iteration, because there are situations where the first iteration is for `test` and the second iteration is for `valid`. Changing seed will be confusing and causes inconsistency. See #712 for related discussion. See #723 for the version with buffer. Differential Revision: [D38675266](https://our.internmc.facebook.com/intern/diff/D38675266) [ghstack-poisoned]

This PR adds RandomSplitter without a buffer. The upside is that this uses less memory (good for memory-bound cases) but the downside are 1) only one group can be iterated through at a time and 2) it skips over all the groups that do not match the target (which is potentially wasteful). Implementation note: * I decided against reusing `_ChildDataPipe` since its features are overly complicated for this use case. * I also decided against having an option to change seed automatically after each iteration, because there are situations where the first iteration is for `test` and the second iteration is for `valid`. Changing seed will be confusing and causes inconsistency. See #712 for related discussion. See #723 for the version with buffer. Differential Revision: [D38675266](https://our.internmc.facebook.com/intern/diff/D38675266) [ghstack-poisoned]