[Feature] update sharded loader #468
Conversation
| batch = batchify_fn([dataset[i] for i in samples]) | ||
| return batch | ||
|
|
||
| class _MultiWorkerIter(object): |
There was a problem hiding this comment.
Why do we have to copy the full implementation?
There was a problem hiding this comment.
1.4.0 not yet released, better save the effort to move dependency forward to a nightly build
There was a problem hiding this comment.
Ok, I will approve it once it passes the ci test.
|
|
||
| """ | ||
| def __init__(self, dataset, batch_size=None, shuffle=False, sampler=None, |
There was a problem hiding this comment.
Similar to previous comment, why do we need to copy all.
|
Job PR-468/6 is complete. |
|
|
||
| @pytest.mark.remote_required | ||
| def test_sharded_data_loader_record_file(): | ||
| if not hasattr(mx.recordio.MXRecordIO, '_check_pid'): | ||
| # skip if mxnet<=1.4.0 detected, some hotfix is not included so recordfile will break | ||
| return |
There was a problem hiding this comment.
How about skipping the test depending on mxnet.__version__? In case there is any change in later mxnet versions that changes the _check_pid attribute it may help keep this test useful?
There was a problem hiding this comment.
Some early 1.4.0 nightly build version don't have the according PR included, so I don't know how to skip those given version only. IMO it's a non issue after the official 1.4.0 is released
* update sharded loader * fix * fix threadpool * use thread_pool, test no merge * fix sharded batch
Description
Update SharededDataLoader according to apache/mxnet#13447
Checklist
Essentials