[DataLoader2] Deep copy DataPipe during initialization

[NivekT]

Stack from ghstack:

• -> [DataLoader2] Deep copy DataPipe during initialization #786

Deep copy DataPipe during initialization to avoid sharing states if the same DataPipe is being passed to multiple usages.

This can be merged before or after #746 (the other PR will have to rebase).

Differential Revision: D39741743

---

BC Breaking Note:

Previously, if a DataPipe is being passed to multiple DataLoaders, the DataPipe's state can be altered by any of those DataLoaders. In some cases, that may raise exception due to the single iterator constraint; in other cases, some behaviors can be changed due to the adapters (e.g. shuffling) of another DataLoader.

Consider the code snippet below:

dp = IterableWrapper([0, 1, 2, 3, 4])
dl1 = DataLoader2(dp)
dl2 = DataLoader2(dp)
for x, y in zip(dl1, dl2):
    print(x, y)

Prior to this change, since the DataPipe objects are identical for each DataLoader2, it would've violated the single iterator constraint and raised an exception.

RuntimeError: This iterator has been invalidated because another iterator has been created from the same IterDataPipe...

After this change, since the DataPipe objects are deep copied within the DataLoader2s, they are no longer the same object within each. You can iterate through them simultaneously without issue:

0 0
1 1
2 2
3 3
4 4

[NivekT]

@NivekT has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[NivekT]

@NivekT has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[NivekT]

@NivekT has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[NivekT]

@NivekT has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ejguan]

@NivekT Do you think we need to add BC breaking note to this PR?

[NivekT]

@NivekT Do you think we need to add BC breaking note to this PR?

Actually yes, I think we should, even though I don't think users would intentionally rely on the state of DataPipes being shared among DataLoaders.

[ejguan]

@NivekT Do you think we need to add BC breaking note to this PR?

Actually yes, I think we should, even though I don't think users would intentionally rely on the state of DataPipes being shared among DataLoaders.

Can you add a brief example in the bc breaking note that the same DataPipe used by multiple DLv2?

[NivekT]

Will do by the afternoon.

[ejguan]

Will do by the afternoon.

Thank you so much!

[NivekT]

@ejguan Updated this PR's description.