Skip to content
This repository was archived by the owner on Jan 15, 2024. It is now read-only.

remove _fork - #353

Merged
szha merged 17 commits into
dmlc:masterfrom
zhreshold:loader
Oct 23, 2018
Merged

remove _fork#353
szha merged 17 commits into
dmlc:masterfrom
zhreshold:loader

Conversation

@zhreshold

Copy link
Copy Markdown
Member

Description

Fix shared Dataloader after removing _fork() in upstream mxnet. apache/mxnet#12554

Checklist

Essentials

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented
  • Interesting edge cases to note here

@zhreshold
zhreshold requested a review from szha as a code owner October 1, 2018 23:55
@szha
szha requested a review from szhengac October 2, 2018 03:34
@szhengac

szhengac commented Oct 2, 2018

Copy link
Copy Markdown
Member

The mxnet version in env needs to be updated.

@szha

szha commented Oct 2, 2018

Copy link
Copy Markdown
Member

Let's hold onto this change so that we can release 0.4.1 w/ mxnet 1.3.0.

@mli

mli commented Oct 2, 2018

Copy link
Copy Markdown
Member

Job PR-353/2 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-353/2/index.html

@mli

mli commented Oct 9, 2018

Copy link
Copy Markdown
Member

Job PR-353/3 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-353/3/index.html

@mli

mli commented Oct 16, 2018

Copy link
Copy Markdown
Member

Job PR-353/7 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-353/7/index.html

@codecov

codecov Bot commented Oct 16, 2018

Copy link
Copy Markdown

Codecov Report

Merging #353 into master will decrease coverage by 1%.
The diff coverage is 31.25%.

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
- Coverage   62.47%   61.46%   -1.01%     
==========================================
  Files          84       84              
  Lines        7088     7069      -19     
  Branches     1122     1116       -6     
==========================================
- Hits         4428     4345      -83     
- Misses       2357     2436      +79     
+ Partials      303      288      -15
Flag Coverage Δ
#PR353 61.46% <31.25%> (-0.85%) ⬇️
#PR370 ?
#master ?
#py2 61.26% <31.25%> (-1%) ⬇️
#py3 61.23% <31.25%> (-0.07%) ⬇️

Comment thread tests/unittest/train/test_dataloader.py Outdated
import mxnet as mx
from gluonnlp.data import FixedBucketSampler, ShardedDataLoader
from mxnet import gluon
from mxnet.test_utils import download

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from mxnet.test_utils import download
from mxnet.gluon.utils import download

@@ -2,6 +2,7 @@
import mxnet as mx

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import mxnet as mx
import os
import mxnet as mx

Comment thread tests/unittest/train/test_dataloader.py Outdated
# test record file
url_format = 'https://apache-mxnet.s3-accelerate.amazonaws.com/gluon/dataset/{}'
filename = 'not_hotdog_validation-c0201740.rec'
download(url_format.format(filename))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
download(url_format.format(filename))
download(url_format.format(filename), path=os.path.join('tests', 'data', filename))

Comment thread tests/unittest/train/test_dataloader.py Outdated
url_format = 'https://apache-mxnet.s3-accelerate.amazonaws.com/gluon/dataset/{}'
filename = 'not_hotdog_validation-c0201740.rec'
download(url_format.format(filename))
rec_dataset = gluon.data.RecordFileDataset(filename)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rec_dataset = gluon.data.RecordFileDataset(filename)
rec_dataset = gluon.data.RecordFileDataset(os.path.join('tests', 'data', filename))

@szha szha added the release focus Progress focus for release label Oct 20, 2018
@mli

mli commented Oct 23, 2018

Copy link
Copy Markdown
Member

Job PR-353/18 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-353/18/index.html

@szha
szha merged commit ab24ce6 into dmlc:master Oct 23, 2018
@zhreshold
zhreshold deleted the loader branch October 23, 2018 21:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release focus Progress focus for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants