Update sharded dataloader based on the change of dataloader #280
Conversation
|
Can you shortly describe what caused the deadlock? Was it due to upstream mxnet change? Nevermind, I saw the discussion in #274 |
| from mxnet.gluon.data.dataloader import Queue, SimpleQueue, DataLoader, _MultiWorkerIter | ||
| import threading | ||
| from mxnet import context | ||
| from mxnet.gluon.data.dataloader import Queue, SimpleQueue, DataLoader, \ |
There was a problem hiding this comment.
I suggest to put them in the iterator otherwise gluonnlp will instantly depend on the latest mxnet master
There was a problem hiding this comment.
Or we can let DataLoader takes function worker_loop as an argument in init function. This can makes the code cleaner.
There was a problem hiding this comment.
@zhreshold gluon-nlp already depends on mxnet master, so it's no problem that this change also requires mxnet master. As far as I understand, your change in mxnet will be part of the 1.3 release?
After the coming release, it would be great if we can reduce the duplication of DataLoader code in gluonnlp. Does @szhengac's suggestion sound good to you?
There was a problem hiding this comment.
I would agree to add an more customizable option to data loader iter, in terms of every aspect.
@szhengac Yes you are good to go with the mxnet, let me know if you have concerns.
|
@szhengac mentioned that he'd add the option upstream to mxnet and include that in the fix. |
5 participants
Description
Fix #274
Checklist
Essentials
Changes
Comments