leezu commented on a change in pull request #17841:
URL: https://github.com/apache/incubator-mxnet/pull/17841#discussion_r416823281



##########
File path: ci/build_windows.py
##########
@@ -184,6 +184,7 @@ def windows_build(args):
             if ret != 0:
                 build_try += 1
                 logging.info("{} build(s) have failed".format(build_try))
+                sys.exit(1)

Review comment:
       Shouldn't be added. Why are you adding it?

##########
File path: python/mxnet/gluon/contrib/data/vision/dataloader.py
##########
@@ -0,0 +1,521 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# coding: utf-8
+# pylint: disable= arguments-differ, wildcard-import
+"Contrib Vision DataLoaders."
+import logging
+import numpy as np
+
+from ..... import nd
+from .....util import is_np_array
+from ..... import np as _mx_np   # pylint: disable=reimported
+from ....nn import HybridSequential, Sequential, HybridBlock, Block
+from ....data.vision import transforms
+from ....data import DataLoader
+from .transforms import bbox
+
+__all__ = ['create_image_augment', 'ImageDataLoader', 'ImageBboxDataLoader']
+
+def create_image_augment(data_shape, resize=0, rand_crop=False, 
rand_resize=False, rand_mirror=False,
+                         mean=None, std=None, brightness=0, contrast=0, 
saturation=0, hue=0,
+                         pca_noise=0, rand_gray=0, inter_method=2, 
dtype='float32'):
+    """Creates an augmenter block.
+
+    Parameters
+    ----------
+    data_shape : tuple of int
+        Shape for output data
+    resize : int
+        Resize shorter edge if larger than 0 at the begining
+    rand_crop : bool
+        Whether to enable random cropping other than center crop
+    rand_resize : bool
+        Whether to enable random sized cropping, require rand_crop to be 
enabled
+    rand_gray : float
+        [0, 1], probability to convert to grayscale for all channels, the 
number
+        of channels will not be reduced to 1
+    rand_mirror : bool
+        Whether to apply horizontal flip to image with probability 0.5
+    mean : np.ndarray or None
+        Mean pixel values for [r, g, b]
+    std : np.ndarray or None
+        Standard deviations for [r, g, b]
+    brightness : float
+        Brightness jittering range (percent)
+    contrast : float
+        Contrast jittering range (percent)
+    saturation : float
+        Saturation jittering range (percent)
+    hue : float
+        Hue jittering range (percent)
+    pca_noise : float
+        Pca noise level (percent)
+    inter_method : int, default=2(Area-based)
+        Interpolation method for all resizing operations
+
+        Possible values:
+        0: Nearest Neighbors Interpolation.
+        1: Bilinear interpolation.
+        2: Bicubic interpolation over 4x4 pixel neighborhood.
+        3: Area-based (resampling using pixel area relation). It may be a
+        preferred method for image decimation, as it gives moire-free
+        results. But when the image is zoomed, it is similar to the Nearest
+        Neighbors method. (used by default).
+        4: Lanczos interpolation over 8x8 pixel neighborhood.
+        10: Random select from interpolation method metioned above.
+        Note:
+        When shrinking an image, it will generally look best with AREA-based
+        interpolation, whereas, when enlarging an image, it will generally 
look best
+        with Bicubic (slow) or Bilinear (faster but still looks OK).
+
+    Examples
+    --------
+    >>> # An example of creating multiple augmenters
+    >>> augs = mx.gluon.contrib.data.create_image_augment(data_shape=(3, 300, 
300), rand_mirror=True,
+    ...    mean=True, brightness=0.125, contrast=0.125, rand_gray=0.05,
+    ...    saturation=0.125, pca_noise=0.05, inter_method=10)
+    """
+    if inter_method == 10:
+        inter_method = np.random.randint(0, 5)
+    augmenter = HybridSequential('default_img_augment_')
+    if resize > 0:
+        augmenter.add(transforms.image.Resize(resize, 
interpolation=inter_method))
+    crop_size = (data_shape[2], data_shape[1])
+    if rand_resize:
+        assert rand_crop
+        augmenter.add(transforms.image.RandomResizedCrop(crop_size, 
interpolation=inter_method))
+    elif rand_crop:
+        augmenter.add(transforms.image.RandomCrop(crop_size, 
interpolation=inter_method))
+    else:
+        augmenter.add(transforms.image.CenterCrop(crop_size, 
interpolation=inter_method))
+
+    if rand_mirror:
+        augmenter.add(transforms.image.RandomFlipLeftRight(0.5))
+
+    augmenter.add(transforms.Cast())
+
+    if brightness or contrast or saturation or hue:
+        augmenter.add(transforms.image.RandomColorJitter(brightness, contrast, 
saturation, hue))
+
+    if pca_noise > 0:
+        augmenter.add(transforms.image.RandomLighting(pca_noise))
+
+    if rand_gray > 0:
+        augmenter.add(transforms.image.RandomGray(rand_gray))
+
+    if mean is True:
+        mean = [123.68, 116.28, 103.53]
+    elif mean is not None:
+        assert isinstance(mean, (tuple, list))
+
+    if std is True:
+        std = [58.395, 57.12, 57.375]
+    elif std is not None:
+        assert isinstance(std, (tuple, list))
+
+    augmenter.add(transforms.image.ToTensor())
+
+    if mean is not None or std is not None:
+        augmenter.add(transforms.image.Normalize(mean, std))
+
+    augmenter.add(transforms.Cast(dtype))
+
+    return augmenter
+
+class ImageDataLoader(object):

Review comment:
       This seems to be mostly equivalent to using normal `DataLoader` and 
calling `dataset.transform_first(augmenter)`? Would it be easier for users and 
more maintainable to just use `DataLoader`?

##########
File path: python/mxnet/gluon/data/dataloader.py
##########
@@ -572,16 +564,26 @@ def default_batchify_fn(data):
         unless you are experiencing timeout and you know it's due to slow data 
loading.
         Sometimes full `shared_memory` will cause all workers to hang and 
causes timeout. In these
         cases please reduce `num_workers` or increase system `shared_memory` 
size instead.
+    try_nopython : bool, default is None

Review comment:
       Type should be bool or None

##########
File path: python/mxnet/gluon/data/dataloader.py
##########
@@ -572,16 +564,26 @@ def default_batchify_fn(data):
         unless you are experiencing timeout and you know it's due to slow data 
loading.
         Sometimes full `shared_memory` will cause all workers to hang and 
causes timeout. In these
         cases please reduce `num_workers` or increase system `shared_memory` 
size instead.
+    try_nopython : bool, default is None
+        Try compile python dataloading pipeline into pure MXNet c++ 
implementation. The benefit is
+        potentially faster iteration, no `shared_memory` usage, and less 
processes managed by python.
+        The compilation is not gauranteed to support all use cases, but it 
will fallback to python in
+        case of failure. You can set `try_nopython` to `False` to disable 
auto-detection of the
+        compilation feature or leave it to `None` to allow MXNet to determine 
it automatically.
+        If you request `try_nopython` to `True` and the compilation fails, it 
will raise a warning and

Review comment:
       It seems inconsistent? Should `True` raise an exception instead if it 
fails?

##########
File path: python/mxnet/gluon/data/dataloader.py
##########
@@ -607,28 +609,51 @@ def __init__(self, dataset, batch_size=None, 
shuffle=False, sampler=None,
         self._num_workers = num_workers if num_workers >= 0 else 0
         self._worker_pool = None
         self._prefetch = max(0, int(prefetch) if prefetch is not None else 2 * 
self._num_workers)
-        if self._num_workers > 0:
-            if self._thread_pool:
-                self._worker_pool = ThreadPool(self._num_workers,
-                                               
initializer=_thread_worker_initializer,
-                                               initargs=(is_np_shape(), 
is_np_array()))
-            else:
-                # set ignore keyboard interupt signal before forking processes
-                original_sigint_handler = signal.signal(signal.SIGINT, 
signal.SIG_IGN)
-                self._worker_pool = multiprocessing.Pool(
-                    self._num_workers, initializer=_worker_initializer,
-                    initargs=[self._dataset, is_np_shape(), is_np_array()])
-                # resume keyboard interupt signal in main process
-                signal.signal(signal.SIGINT, original_sigint_handler)
         if batchify_fn is None:
             if num_workers > 0:
-                self._batchify_fn = default_mp_batchify_fn
+                self._batchify_fn = _batchify.Stack(use_shared_mem=True)
             else:
-                self._batchify_fn = default_batchify_fn
+                self._batchify_fn = _batchify.Stack()
         else:
             self._batchify_fn = batchify_fn
 
+        if num_workers > 0 and (try_nopython or try_nopython is None):

Review comment:
       `nopython` shouldn't depend on `num_workers`? Even a single threaded 
nopython iter is helpful

##########
File path: python/mxnet/gluon/data/dataloader.py
##########
@@ -607,28 +609,51 @@ def __init__(self, dataset, batch_size=None, 
shuffle=False, sampler=None,
         self._num_workers = num_workers if num_workers >= 0 else 0
         self._worker_pool = None
         self._prefetch = max(0, int(prefetch) if prefetch is not None else 2 * 
self._num_workers)
-        if self._num_workers > 0:
-            if self._thread_pool:
-                self._worker_pool = ThreadPool(self._num_workers,
-                                               
initializer=_thread_worker_initializer,
-                                               initargs=(is_np_shape(), 
is_np_array()))
-            else:
-                # set ignore keyboard interupt signal before forking processes
-                original_sigint_handler = signal.signal(signal.SIGINT, 
signal.SIG_IGN)
-                self._worker_pool = multiprocessing.Pool(
-                    self._num_workers, initializer=_worker_initializer,
-                    initargs=[self._dataset, is_np_shape(), is_np_array()])
-                # resume keyboard interupt signal in main process
-                signal.signal(signal.SIGINT, original_sigint_handler)
         if batchify_fn is None:
             if num_workers > 0:
-                self._batchify_fn = default_mp_batchify_fn
+                self._batchify_fn = _batchify.Stack(use_shared_mem=True)
             else:
-                self._batchify_fn = default_batchify_fn
+                self._batchify_fn = _batchify.Stack()
         else:
             self._batchify_fn = batchify_fn
 
+        if num_workers > 0 and (try_nopython or try_nopython is None):
+            # check for capability to use mx backend threadedLoader
+            use_mx_iter, mx_iter_args = _check_mx_loader_capability(
+                self._dataset, self._batch_sampler, self._batchify_fn)
+            if not use_mx_iter:
+                if try_nopython:
+                    warnings.warn(mx_iter_args)
+        else:
+            use_mx_iter = False
+
+        if use_mx_iter:
+            logging.info("Using MXNet backend ThreadedDataLoader with %s 
workers "
+                         "instead of python dataloader.", self._num_workers)
+            self._mx_iter = MXThreadedDataLoader(
+                num_workers=self._num_workers,
+                pin_memory=self._pin_memory,
+                pin_device_id=self._pin_device_id,
+                prefetch=self._prefetch, **mx_iter_args)
+        else:
+            if self._num_workers > 0:

Review comment:
       Where is the `self._num_workers == 0` case handled?

##########
File path: python/mxnet/gluon/data/dataloader.py
##########
@@ -655,3 +680,119 @@ def __del__(self):
             # https://bugs.python.org/issue34172
             assert isinstance(self._worker_pool, multiprocessing.pool.Pool)
             self._worker_pool.terminate()
+
+def _check_mx_loader_capability(dataset, batch_sampler, batchify_fn):
+    from ._internal import MXDataset, MXSampler
+    from ._internal import MXBatchifyFunction
+    mx_loader_args = {}
+    error_template = "MXNet backend loader compatibility: " \
+        "[dataset - {}][batchify_fn - {}][batch sampler - {}]"
+
+    # supported dataset
+    if isinstance(dataset, MXDataset):
+        mx_loader_args['dataset'] = dataset
+    elif hasattr(dataset, '__mx_handle__'):
+        try:
+            mx_loader_args['dataset'] = dataset.__mx_handle__()
+        except NotImplementedError:
+            return False, error_template.format('fail', 'unknown', 'unknown')
+    else:
+        return False, error_template.format('fail', 'unknown', 'unknown')
+
+    # supported batchify functions
+    if hasattr(batchify_fn, '__mx_handle__'):
+        mx_loader_args['batchify_fn'] = batchify_fn.__mx_handle__()
+    elif isinstance(batchify_fn, MXBatchifyFunction):
+        mx_loader_args['batchify_fn'] = batchify_fn
+    else:
+        return False, error_template.format('pass', 'fail', 'unknown')
+
+    # supported sampler
+    if isinstance(batch_sampler, _sampler.BatchSampler):
+        if isinstance(batch_sampler._sampler, _sampler.SequentialSampler):
+            mx_loader_args['batch_sampler'] = MXSampler(
+                'SequentialSampler', length=batch_sampler._sampler._length,
+                start=batch_sampler._sampler._start,
+                batch_size=batch_sampler._batch_size,
+                last_batch=batch_sampler._last_batch)
+        elif isinstance(batch_sampler._sampler, _sampler.RandomSampler):
+            mx_loader_args['batch_sampler'] = MXSampler(
+                'RandomSampler', length=batch_sampler._sampler._length,
+                batch_size=batch_sampler._batch_size,
+                last_batch=batch_sampler._last_batch)
+        else:
+            return False, error_template.format('pass', 'pass', 'fail')
+    elif isinstance(batch_sampler, MXSampler):
+        mx_loader_args['batch_sampler'] = batch_sampler
+    else:
+        return False, error_template.format('pass', 'pass', 'fail')
+    # all good
+    return True, mx_loader_args
+
+
+class MXThreadedDataLoader(object):

Review comment:
       This should be private and not exposed as API?

##########
File path: src/io/iter_sampler.cc
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2020 by Contributors
+ * \file iter_sampler.cc
+ * \brief The sampler iterator for access dataset elements.
+ */
+#include <dmlc/parameter.h>
+#include <mshadow/random.h>
+#include <mxnet/io.h>
+#include <mxnet/base.h>
+#include <mxnet/resource.h>
+#include <numeric>
+#include "../common/utils.h"
+#include "./iter_batchloader.h"
+#include "./iter_prefetcher.h"
+
+namespace mxnet {
+namespace io {
+struct SequentialSamplerParam : public dmlc::Parameter<SequentialSamplerParam> 
{
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int start;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(SequentialSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(start).set_default(0)
+          .describe("Start of the index.");
+  }
+};  // struct SequentialSamplerParam
+
+DMLC_REGISTER_PARAMETER(SequentialSamplerParam);
+
+class SequentialSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here

Review comment:
       Should we fix `DataBatch` instead? In general it appears that 
`DataBatch` has places severe limitations on the data pipeline, ie. having 
exactly two arrays in every batch. One data array and one label array.

##########
File path: src/io/iter_sampler.cc
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2020 by Contributors
+ * \file iter_sampler.cc
+ * \brief The sampler iterator for access dataset elements.
+ */
+#include <dmlc/parameter.h>
+#include <mshadow/random.h>
+#include <mxnet/io.h>
+#include <mxnet/base.h>
+#include <mxnet/resource.h>
+#include <numeric>
+#include "../common/utils.h"
+#include "./iter_batchloader.h"
+#include "./iter_prefetcher.h"
+
+namespace mxnet {
+namespace io {
+struct SequentialSamplerParam : public dmlc::Parameter<SequentialSamplerParam> 
{
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int start;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(SequentialSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(start).set_default(0)
+          .describe("Start of the index.");
+  }
+};  // struct SequentialSamplerParam
+
+DMLC_REGISTER_PARAMETER(SequentialSamplerParam);
+
+class SequentialSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here
+    out_.data[1] = TBlob(indices_.data(), TShape({1, }), cpu::kDevMask, 0);
+  }
+
+  virtual void BeforeFirst(void) {
+    pos_ = 0;
+  }
+
+  virtual int64_t GetLenHint(void) const {
+    return static_cast<int64_t>(indices_.size());
+  }
+
+  virtual bool Next(void) {
+    if (pos_ < indices_.size()) {
+      int64_t *ptr = indices_.data() + pos_;
+      out_.data[0] = TBlob(ptr, TShape({1, }), cpu::kDevMask, 0);
+      ++pos_;
+      return true;
+    }
+    return false;
+  }
+
+  virtual const DataInst &Value(void) const {
+    return out_;
+  }
+
+ private:
+  /*! \brief Stored integer indices */
+  std::vector<int64_t> indices_;
+  /*! \brief current position for iteration */
+  std::size_t pos_;
+  /*! \brief data for next value */
+  DataInst out_;
+  /*! \brief arguments */
+  SequentialSamplerParam param_;
+};  // class SequentialSampler
+
+MXNET_REGISTER_IO_ITER(SequentialSampler)
+.describe(R"code(Returns the sequential sampler iterator.
+)code" ADD_FILELINE)
+.add_arguments(SequentialSamplerParam::__FIELDS__())
+.add_arguments(BatchSamplerParam::__FIELDS__())
+.set_body([]() {
+    return
+        new BatchSampler(
+            new SequentialSampler());
+  });
+
+struct RandomSamplerParam : public dmlc::Parameter<RandomSamplerParam> {
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int seed;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(RandomSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(seed).set_default(0)
+          .describe("Random seed.");
+  }
+};  // struct RandomSamplerParam
+
+DMLC_REGISTER_PARAMETER(RandomSamplerParam);
+
+class RandomSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    rng_.reset(new common::RANDOM_ENGINE(kRandMagic + param_.seed));
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here
+    out_.data[1] = TBlob(indices_.data(), TShape({1, }), cpu::kDevMask, 0);
+    BeforeFirst();
+  }
+
+  virtual void BeforeFirst(void) {
+    std::shuffle(std::begin(indices_), std::end(indices_), *rng_);
+    pos_ = 0;
+  }
+
+  virtual int64_t GetLenHint(void) const {
+    return static_cast<int64_t>(indices_.size());
+  }
+
+  virtual bool Next(void) {
+    if (pos_ < indices_.size()) {
+      int64_t *ptr = indices_.data() + pos_;
+      out_.data[0] = TBlob(ptr, TShape({1, }), cpu::kDevMask, 0);
+      ++pos_;
+      return true;
+    }
+    return false;
+  }
+
+  virtual const DataInst &Value(void) const {
+    return out_;
+  }
+ private:
+  /*! \brief random magic number */
+  static const int kRandMagic = 2333;
+  /*! \brief Stored integer indices */
+  std::vector<int64_t> indices_;
+  /*! \brief current position for iteration */
+  std::size_t pos_;
+  /*! \brief data for next value */
+  DataInst out_;
+  /*! \brief random generator engine */
+  std::unique_ptr<common::RANDOM_ENGINE> rng_;

Review comment:
       Let's just use `std::mt19937` instead of `RANDOM_ENGINE` here, to make 
the code easier to read?

##########
File path: src/io/iter_prefetcher.h
##########
@@ -86,14 +88,20 @@ class PrefetcherIter : public IIterator<DataBatch> {
             auto dtype = param_.dtype
                              ? param_.dtype.value()
                              : batch.data[i].type_flag_;
+            auto ctx = ((param_.ctx == PrefetcherParam::kCPUPinned) && 
(param_.device_id >= 0)) ?
+              Context::CPUPinned(param_.device_id) : Context::CPU();
             (*dptr)->data.at(i) = NDArray(batch.data[i].shape_,
-                                          Context::CPU(), false,
+                                          ctx, false,
                                           dtype);
           }
         }
         CHECK(batch.data.size() == (*dptr)->data.size());
         // copy data over
         for (size_t i = 0; i < batch.data.size(); ++i) {
+          if ((*dptr)->data.at(i).shape() != batch.data[i].shape_) {
+            // perf warning, dynamic buffer might be slow

Review comment:
       This seems to be a TODO?

##########
File path: src/io/iter_sampler.cc
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2020 by Contributors
+ * \file iter_sampler.cc
+ * \brief The sampler iterator for access dataset elements.
+ */
+#include <dmlc/parameter.h>
+#include <mshadow/random.h>
+#include <mxnet/io.h>
+#include <mxnet/base.h>
+#include <mxnet/resource.h>
+#include <numeric>
+#include "../common/utils.h"
+#include "./iter_batchloader.h"
+#include "./iter_prefetcher.h"
+
+namespace mxnet {
+namespace io {
+struct SequentialSamplerParam : public dmlc::Parameter<SequentialSamplerParam> 
{
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int start;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(SequentialSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(start).set_default(0)
+          .describe("Start of the index.");
+  }
+};  // struct SequentialSamplerParam
+
+DMLC_REGISTER_PARAMETER(SequentialSamplerParam);
+
+class SequentialSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here
+    out_.data[1] = TBlob(indices_.data(), TShape({1, }), cpu::kDevMask, 0);
+  }
+
+  virtual void BeforeFirst(void) {
+    pos_ = 0;
+  }
+
+  virtual int64_t GetLenHint(void) const {
+    return static_cast<int64_t>(indices_.size());
+  }
+
+  virtual bool Next(void) {
+    if (pos_ < indices_.size()) {
+      int64_t *ptr = indices_.data() + pos_;
+      out_.data[0] = TBlob(ptr, TShape({1, }), cpu::kDevMask, 0);
+      ++pos_;
+      return true;
+    }
+    return false;
+  }
+
+  virtual const DataInst &Value(void) const {
+    return out_;
+  }
+
+ private:
+  /*! \brief Stored integer indices */
+  std::vector<int64_t> indices_;
+  /*! \brief current position for iteration */
+  std::size_t pos_;
+  /*! \brief data for next value */
+  DataInst out_;
+  /*! \brief arguments */
+  SequentialSamplerParam param_;
+};  // class SequentialSampler
+
+MXNET_REGISTER_IO_ITER(SequentialSampler)
+.describe(R"code(Returns the sequential sampler iterator.
+)code" ADD_FILELINE)
+.add_arguments(SequentialSamplerParam::__FIELDS__())
+.add_arguments(BatchSamplerParam::__FIELDS__())
+.set_body([]() {
+    return
+        new BatchSampler(
+            new SequentialSampler());
+  });
+
+struct RandomSamplerParam : public dmlc::Parameter<RandomSamplerParam> {
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int seed;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(RandomSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(seed).set_default(0)
+          .describe("Random seed.");
+  }
+};  // struct RandomSamplerParam
+
+DMLC_REGISTER_PARAMETER(RandomSamplerParam);
+
+class RandomSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    rng_.reset(new common::RANDOM_ENGINE(kRandMagic + param_.seed));

Review comment:
       Use mxnet's seed instead of introducing new seed for random sampler?

##########
File path: tests/python/unittest/test_numpy_interoperability.py
##########
@@ -1832,7 +1832,7 @@ def test_scalar_output():
             v2 = vec2.astype(dt)
             OpArgMngr.add_workload('matmul', v1, v2)
             OpArgMngr.add_workload('matmul', v2.T, v1)
-
+    

Review comment:
       Why add the whitespace here (and many other places in this file).

##########
File path: src/io/iter_sampler.cc
##########
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ *  Copyright (c) 2020 by Contributors
+ * \file iter_sampler.cc
+ * \brief The sampler iterator for access dataset elements.
+ */
+#include <dmlc/parameter.h>
+#include <mshadow/random.h>
+#include <mxnet/io.h>
+#include <mxnet/base.h>
+#include <mxnet/resource.h>
+#include <numeric>
+#include "../common/utils.h"
+#include "./iter_batchloader.h"
+#include "./iter_prefetcher.h"
+
+namespace mxnet {
+namespace io {
+struct SequentialSamplerParam : public dmlc::Parameter<SequentialSamplerParam> 
{
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int start;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(SequentialSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(start).set_default(0)
+          .describe("Start of the index.");
+  }
+};  // struct SequentialSamplerParam
+
+DMLC_REGISTER_PARAMETER(SequentialSamplerParam);
+
+class SequentialSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here
+    out_.data[1] = TBlob(indices_.data(), TShape({1, }), cpu::kDevMask, 0);
+  }
+
+  virtual void BeforeFirst(void) {
+    pos_ = 0;
+  }
+
+  virtual int64_t GetLenHint(void) const {
+    return static_cast<int64_t>(indices_.size());
+  }
+
+  virtual bool Next(void) {
+    if (pos_ < indices_.size()) {
+      int64_t *ptr = indices_.data() + pos_;
+      out_.data[0] = TBlob(ptr, TShape({1, }), cpu::kDevMask, 0);
+      ++pos_;
+      return true;
+    }
+    return false;
+  }
+
+  virtual const DataInst &Value(void) const {
+    return out_;
+  }
+
+ private:
+  /*! \brief Stored integer indices */
+  std::vector<int64_t> indices_;
+  /*! \brief current position for iteration */
+  std::size_t pos_;
+  /*! \brief data for next value */
+  DataInst out_;
+  /*! \brief arguments */
+  SequentialSamplerParam param_;
+};  // class SequentialSampler
+
+MXNET_REGISTER_IO_ITER(SequentialSampler)
+.describe(R"code(Returns the sequential sampler iterator.
+)code" ADD_FILELINE)
+.add_arguments(SequentialSamplerParam::__FIELDS__())
+.add_arguments(BatchSamplerParam::__FIELDS__())
+.set_body([]() {
+    return
+        new BatchSampler(
+            new SequentialSampler());
+  });
+
+struct RandomSamplerParam : public dmlc::Parameter<RandomSamplerParam> {
+  /*! \brief Length of the sequence. */
+  size_t length;
+  /*! \brief Random seed.*/
+  int seed;
+  // declare parameters
+  DMLC_DECLARE_PARAMETER(RandomSamplerParam) {
+      DMLC_DECLARE_FIELD(length)
+          .describe("Length of the sequence.");
+      DMLC_DECLARE_FIELD(seed).set_default(0)
+          .describe("Random seed.");
+  }
+};  // struct RandomSamplerParam
+
+DMLC_REGISTER_PARAMETER(RandomSamplerParam);
+
+class RandomSampler : public IIterator<DataInst> {
+ public:
+  virtual void Init(const std::vector<std::pair<std::string, std::string> >& 
kwargs) {
+    param_.InitAllowUnknown(kwargs);
+    indices_.resize(param_.length);
+    std::iota(std::begin(indices_), std::end(indices_), 0);  // fill like 
arange
+    rng_.reset(new common::RANDOM_ENGINE(kRandMagic + param_.seed));
+    out_.data.resize(2);  // label required by DataBatch, we can use fake 
label here
+    out_.data[1] = TBlob(indices_.data(), TShape({1, }), cpu::kDevMask, 0);
+    BeforeFirst();
+  }
+
+  virtual void BeforeFirst(void) {
+    std::shuffle(std::begin(indices_), std::end(indices_), *rng_);
+    pos_ = 0;
+  }
+
+  virtual int64_t GetLenHint(void) const {
+    return static_cast<int64_t>(indices_.size());
+  }
+
+  virtual bool Next(void) {
+    if (pos_ < indices_.size()) {
+      int64_t *ptr = indices_.data() + pos_;
+      out_.data[0] = TBlob(ptr, TShape({1, }), cpu::kDevMask, 0);
+      ++pos_;
+      return true;
+    }
+    return false;
+  }
+
+  virtual const DataInst &Value(void) const {
+    return out_;
+  }
+ private:
+  /*! \brief random magic number */
+  static const int kRandMagic = 2333;

Review comment:
       Why have a random magic number?

##########
File path: tests/python/unittest/test_contrib_gluon_data_vision.py
##########
@@ -0,0 +1,145 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import mxnet as mx
+import numpy as np
+import scipy.ndimage
+from mxnet.test_utils import *
+from common import assertRaises, with_seed
+import shutil
+import tempfile
+import unittest
+
+def _get_data(url, dirname):
+    import os, tarfile
+    download(url, dirname=dirname, overwrite=False)
+    fname = os.path.join(dirname, url.split('/')[-1])
+    tar = tarfile.open(fname)
+    source_images = [os.path.join(dirname, x.name) for x in tar.getmembers() 
if x.isfile()]
+    if len(source_images) < 1 or not os.path.isfile(source_images[0]):
+        # skip extracting if exists
+        tar.extractall(path=dirname)
+    tar.close()
+    return source_images
+
+def _generate_objects():
+    num = np.random.randint(1, 10)
+    xy = np.random.rand(num, 2)
+    wh = np.random.rand(num, 2) / 2
+    left = (xy[:, 0] - wh[:, 0])[:, np.newaxis]
+    right = (xy[:, 0] + wh[:, 0])[:, np.newaxis]
+    top = (xy[:, 1] - wh[:, 1])[:, np.newaxis]
+    bot = (xy[:, 1] + wh[:, 1])[:, np.newaxis]
+    boxes = np.maximum(0., np.minimum(1., np.hstack((left, top, right, bot))))
+    cid = np.random.randint(0, 20, size=num)
+    label = np.hstack((cid[:, np.newaxis], boxes)).ravel().tolist()
+    return [2, 5] + label
+
+
+class TestImage(unittest.TestCase):

Review comment:
       Did you check this is completely covered by pytest's unittest support 
https://docs.pytest.org/en/latest/unittest.html ?

##########
File path: tests/python/unittest/test_numpy_contrib_gluon_data_vision.py
##########
@@ -0,0 +1,146 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import mxnet as mx
+import numpy as np
+import scipy.ndimage
+from mxnet.test_utils import *
+from common import assertRaises, with_seed
+import shutil
+import tempfile
+import unittest
+
+def _get_data(url, dirname):
+    import os, tarfile
+    download(url, dirname=dirname, overwrite=False)
+    fname = os.path.join(dirname, url.split('/')[-1])
+    tar = tarfile.open(fname)
+    source_images = [os.path.join(dirname, x.name) for x in tar.getmembers() 
if x.isfile()]
+    if len(source_images) < 1 or not os.path.isfile(source_images[0]):
+        # skip extracting if exists
+        tar.extractall(path=dirname)
+    tar.close()
+    return source_images
+
+def _generate_objects():
+    num = np.random.randint(1, 10)
+    xy = np.random.rand(num, 2)
+    wh = np.random.rand(num, 2) / 2
+    left = (xy[:, 0] - wh[:, 0])[:, np.newaxis]
+    right = (xy[:, 0] + wh[:, 0])[:, np.newaxis]
+    top = (xy[:, 1] - wh[:, 1])[:, np.newaxis]
+    bot = (xy[:, 1] + wh[:, 1])[:, np.newaxis]
+    boxes = np.maximum(0., np.minimum(1., np.hstack((left, top, right, bot))))
+    cid = np.random.randint(0, 20, size=num)
+    label = np.hstack((cid[:, np.newaxis], boxes)).ravel().tolist()
+    return [2, 5] + label
+
+
+class TestImage(unittest.TestCase):
+    IMAGES_URL = "http://data.mxnet.io/data/test_images.tar.gz";
+
+    def setUp(self):
+        self.IMAGES_DIR = tempfile.mkdtemp()
+        self.IMAGES = _get_data(self.IMAGES_URL, self.IMAGES_DIR)
+        print("Loaded {} images".format(len(self.IMAGES)))
+
+    def tearDown(self):
+        if self.IMAGES_DIR:
+            print("cleanup {}".format(self.IMAGES_DIR))
+            shutil.rmtree(self.IMAGES_DIR)
+
+    @use_np
+    def test_imageiter(self):
+        im_list = [[np.random.randint(0, 5), x] for x in self.IMAGES]
+        fname = './data/test_imageiter.lst'
+        file_list = ['\t'.join([str(k), str(np.random.randint(0, 5)), x])
+                        for k, x in enumerate(self.IMAGES)]
+        with open(fname, 'w') as f:
+            for line in file_list:
+                f.write(line + '\n')
+
+        test_list = ['imglist', 'path_imglist']
+        for dtype in ['int32', 'float32', 'int64', 'float64']:
+            for test in test_list:
+                imglist = im_list if test == 'imglist' else None
+                path_imglist = fname if test == 'path_imglist' else None
+                imageiter_list = [
+                    mx.gluon.contrib.data.vision.ImageDataLoader(2, (3, 224, 
224), imglist=imglist,
+                        path_imglist=path_imglist, path_root='', dtype=dtype),
+                    mx.gluon.contrib.data.vision.ImageDataLoader(3, (3, 224, 
224), imglist=imglist,
+                        path_imglist=path_imglist, path_root='', dtype=dtype, 
last_batch='discard'),
+                    mx.gluon.contrib.data.vision.ImageDataLoader(3, (3, 224, 
224), imglist=imglist,
+                        path_imglist=path_imglist, path_root='', dtype=dtype, 
last_batch='keep'),
+                    mx.gluon.contrib.data.vision.ImageDataLoader(3, (3, 224, 
224), imglist=imglist,
+                        path_imglist=path_imglist, path_root='', dtype=dtype, 
last_batch='rollover'),
+                    mx.gluon.contrib.data.vision.ImageDataLoader(3, (3, 224, 
224), imglist=imglist, shuffle=True,
+                        path_imglist=path_imglist, path_root='', dtype=dtype, 
last_batch='keep',
+                        rand_crop=1, rand_gray=0.1, rand_mirror=True)
+                ]
+                for it in imageiter_list:
+                    for batch in it:
+                        pass
+
+    @use_np
+    def test_image_bbox_iter(self):
+        im_list = [_generate_objects() + [x] for x in self.IMAGES]
+        det_iter = mx.gluon.contrib.data.vision.ImageBboxDataLoader(2, (3, 
300, 300), imglist=im_list, path_root='')
+        for _ in range(3):
+            for _ in det_iter:
+                pass
+        val_iter = mx.gluon.contrib.data.vision.ImageBboxDataLoader(2, (3, 
300, 300), imglist=im_list, path_root='')
+
+        # test batch_size is not divisible by number of images
+        det_iter = mx.gluon.contrib.data.vision.ImageBboxDataLoader(4, (3, 
300, 300), imglist=im_list, path_root='')
+        for _ in det_iter:
+            pass
+
+        # test file list with last batch handle
+        fname = './data/test_imagedetiter.lst'
+        im_list = [[k] + _generate_objects() + [x] for k, x in 
enumerate(self.IMAGES)]
+        with open(fname, 'w') as f:
+            for line in im_list:
+                line = '\t'.join([str(k) for k in line])
+                f.write(line + '\n')
+
+        imageiter_list = [
+            mx.gluon.contrib.data.vision.ImageBboxDataLoader(2, (3, 400, 400),
+                path_imglist=fname, path_root=''),
+            mx.gluon.contrib.data.vision.ImageBboxDataLoader(3, (3, 400, 400),
+                path_imglist=fname, path_root='', last_batch='discard'),
+            mx.gluon.contrib.data.vision.ImageBboxDataLoader(3, (3, 400, 400),
+                path_imglist=fname, path_root='', last_batch='keep'),
+            mx.gluon.contrib.data.vision.ImageBboxDataLoader(3, (3, 400, 400),
+                path_imglist=fname, path_root='', last_batch='rollover'),
+            mx.gluon.contrib.data.vision.ImageBboxDataLoader(3, (3, 400, 400), 
shuffle=True,
+                path_imglist=fname, path_root='', last_batch='keep')
+        ]
+
+    @use_np
+    def test_bbox_augmenters(self):
+        # only test if all augmenters will work
+        # TODO(Joshua Zhang): verify the augmenter outputs

Review comment:
       Is this resolved?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to