data
data copied to clipboard
pytorch
[DataLoader] Deep copy ReadingService during DL2 initialization
Stack from ghstack:
- #728
- -> #746
By creating a deep copy of the input ReadingService during the initialization of DataLoader2, we allow the instance of ReadingService to store state without worrying about the users passing the same ReadingService to initialize additional DataLoaders.
For instance, PrototypeMultiProcessingReadingService stores processes and datapipes as instance variables. If a single object of that ReadingService is used to initialize multiple DataLoaders, it can led to conflicts and incorrect states. This should no longer be an issue with this change.
Relevant test are added in subsequent PR related to lengths and eventually PrototypeMultiprocessingReadingService.
Differential Revision: D38868731
@nivekt has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
We should put this information into the ReadingService interface file too. To make sure that RS developers know about potential deep copy.
One more though, we deepcopy RS, but we pickle-unpickle DP. I think we need to pick consistent approach.
One more though, we deepcopy RS, but we pickle-unpickle DP. I think we need to pick consistent approach.
Totally agree. And, I support deepcopy because it's more readable and even working with unpickable objects.
One more though, we deepcopy RS, but we pickle-unpickle DP. I think we need to pick consistent approach.
Can you elaborate on this? What make a consistent approach matter here?
Also, it seems like some internal ReadingService cannot be deep-copied. One alternative is for users to pass in a constructor instead of the object itself, but that will change the interface quite a bit.
Also, it seems like some internal ReadingService cannot be deep-copied. One alternative is for users to pass in a constructor instead of the object itself, but that will change the interface quite a bit.
It seems the Error comes from construction of process group in __init__ function. You might be able to move them into initialize function to make it working.
@nivekt has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@nivekt has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@nivekt has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@nivekt has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.