Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Gluon iterator plugin #1683

Merged
merged 7 commits into from
Feb 10, 2020
Merged

Add Gluon iterator plugin #1683

merged 7 commits into from
Feb 10, 2020

Conversation

Kh4L
Copy link
Contributor

@Kh4L Kh4L commented Jan 23, 2020

Signed-off-by: Serge Panev spanev@nvidia.com

Why we need this PR?

We need a simpler and more flexible iterator for the MXNet Gluon API, which doesn't require mxnet.io.DataBatch but only NDArrays. Also, we need the support of sparse batches (with non-uniform shape samples), that DataBatch doesn't allow.

This PR needs to be rebased on #1682

What happened in this PR?

  • What solution was applied:
    Add a new MXNet-Gluon iterator.
  • Key points relevant for the review:
    The newly added iterator.
  • Validation and testing:
    Iterator test added.

@Kh4L Kh4L force-pushed the gluon_plugin branch 2 times, most recently from 2ad2d10 to 5555ff8 Compare January 23, 2020 03:42
@Kh4L Kh4L force-pushed the gluon_plugin branch 2 times, most recently from dbf19c2 to e9a9cee Compare January 28, 2020 18:23
@JanuszL
Copy link
Contributor

JanuszL commented Jan 28, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1095917]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1095917]: BUILD FAILED

@Kh4L
Copy link
Contributor Author

Kh4L commented Jan 28, 2020

@JanuszL
#1682 needs to be merged to build this PR on master

@Kh4L Kh4L requested a review from klecki January 28, 2020 23:44
@Kh4L
Copy link
Contributor Author

Kh4L commented Jan 28, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1096498]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1096498]: BUILD FAILED

Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
Signed-off-by: Serge Panev <spanev@nvidia.com>
@Kh4L
Copy link
Contributor Author

Kh4L commented Jan 29, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1096675]: BUILD STARTED

Signed-off-by: Serge Panev <spanev@nvidia.com>
@Kh4L
Copy link
Contributor Author

Kh4L commented Jan 30, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1100033]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1100033]: BUILD FAILED

Signed-off-by: Serge Panev <spanev@nvidia.com>
@Kh4L
Copy link
Contributor Author

Kh4L commented Feb 5, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1110666]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1110666]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1110666]: BUILD PASSED

@Kh4L Kh4L requested a review from klecki February 5, 2020 22:52
Copy link
Contributor

@klecki klecki left a comment

Choose a reason for hiding this comment

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

I would try to factor out some more code to the base class and make the docs a bit more understandable (and the preparation of the returned batch). Otherwise ok.

Signed-off-by: Serge Panev <spanev@nvidia.com>
Copy link
Contributor

@klecki klecki left a comment

Choose a reason for hiding this comment

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

Only a typo left.
You can also slap a comment that mentions why or what we do to create returned batches

dali/python/nvidia/dali/plugin/mxnet.py Show resolved Hide resolved
@klecki
Copy link
Contributor

klecki commented Feb 10, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1119143]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1119143]: BUILD PASSED

@Kh4L Kh4L merged commit bf0aed9 into NVIDIA:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants