[MXNET-432] Add Foreach #10451

zheng-da · 2018-04-07T01:19:01Z

Description

This PR adds a control flow operator: foreach. It takes a Python function as input and run the function over the elements in the input array. foreach is similar to scan in TensorFlow.

This PR is part of the proposal of adding a set of control flow operators to MXNet.
https://cwiki.apache.org/confluence/display/MXNET/Optimize+dynamic+neural+network+models+with+control+flow+operators

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

marcoabreu · 2018-04-07T13:50:32Z

Very interesting! Our of curiosity, is this operator going to be entirely parallelized since it basically splits the graph into multiple subgraphs or how is the approach here?

zheng-da · 2018-04-07T19:35:23Z

we potentially can parallelize among iterations. most likely, there is dependency between iterations. so parallelization among iterations may not be very effective.

reminisce

Very interesting work! I have some comments and questions. Thanks.

reminisce · 2018-05-16T05:15:15Z

python/mxnet/symbol/contrib.py

+                "the number of output states (%d) should be the same as input states (%d)" \
+                % (len(sym_out[1]), len(init_states))
+
+        if (isinstance(sym_out[0], list)):


No parentheses needed. It would result in coding style error in PyCharm.

reminisce · 2018-05-16T05:16:53Z

python/mxnet/symbol/contrib.py

@@ -91,3 +98,99 @@ def rand_zipfian(true_classes, num_sampled, range_max):
    expected_prob_sampled = ((sampled_cls_fp64 + 2.0) / (sampled_cls_fp64 + 1.0)).log() / log_range
    expected_count_sampled = expected_prob_sampled * num_sampled
    return sampled_classes, expected_count_true, expected_count_sampled
+
+def _get_graph_inputs(subg, name, prefix):


Where is prefix used?

the prefix is used for pruning. this part hasn't been implemented yet. I'll probably implement it in the next PR. This PR is already very large.

reminisce · 2018-05-16T05:20:18Z

python/mxnet/symbol/contrib.py

+        syms.append(s)
+    return syms
+
+def foreach(func, input, init_states, back_prop=False, name="foreach"):


input is a keyword in python. Does it make sense to call it data which can be both singular and plural?

reminisce · 2018-05-16T05:30:47Z

python/mxnet/symbol/contrib.py

+    for in_name in g.list_inputs():
+        assert in_name in gin_names, "The input variable %s can't be found in graph inputs: %s" \
+                % (in_name, str(gin_names))
+        if (in_name in state_names):


No parentheses.

reminisce · 2018-05-16T05:31:05Z

python/mxnet/symbol/contrib.py

+        if (in_name in state_names):
+            ordered_ins.append(states_map[in_name])
+            in_state_locs.append(len(ordered_ins) - 1)
+        elif (in_name in data_names):


Same here. No parentheses.

reminisce · 2018-05-16T06:10:49Z

src/operator/nn/control_flow.cc

+.set_attr<FStatefulComputeEx>("FStatefulComputeEx<cpu>", ForeachComputeExCPU)
+.set_attr<std::string>("key_var_num_args", "num_args")
+.add_argument("fn", "Symbol", "Input graph.")
+.add_argument("inputs", "NDArray-or-Symbol[]",


It's called data by default. Does it make sense to keep the naming aligned?

reminisce · 2018-05-16T21:43:54Z

src/operator/nn/control_flow.cc

+    this->params = params;
+  }
+
+  void Forward(std::vector<NDArray> cinputs,


Why copy std::vector<NDArray>?

I'm not sure about this part. I'll figure it out tomorrow.

reminisce · 2018-05-17T04:55:25Z

src/operator/nn/control_flow.cc

+})
+.set_attr<nnvm::FListInputNames>("FListInputNames",
+    [](const NodeAttrs& attrs) {
+  return std::vector<std::string>{"fn", "data1", "data2"};


Should it be generated by param.num_args?

reminisce · 2018-05-17T05:11:04Z

src/operator/nn/control_flow.cc

+// in, state0, state1, ...
+// We need to reorder them in the same order as the input nodes of the subgraph.
+template<typename T>
+static std::vector<T> ReorderInputs(const std::vector<T> &in, const nnvm::IndexedGraph& idx) {


Where is this used?

reminisce · 2018-05-17T05:20:36Z

src/operator/nn/control_flow.cc

+  }
+};
+
+void ForeachState::Forward(std::vector<NDArray> cinputs,


Why copy std::vector<NDArray>?

reminisce · 2018-05-18T07:13:06Z

src/operator/nn/control_flow.cc

+    shape_inputs[loc] = TShape(in_shape->at(loc).begin() + 1, in_shape->at(loc).end());
+  }
+  CHECK_EQ(attrs.subgraphs.size(), 1U);
+  auto g = std::make_shared<nnvm::Graph>();


Seems shared_ptr<Graph> is not necessary here. Graph g would suffice, right?

reminisce · 2018-05-18T07:14:01Z

src/operator/nn/control_flow.cc

+  const auto& idx = g->indexed_graph();
+  CHECK_EQ(idx.input_nodes().size(), in_shape->size());
+  CHECK_EQ(idx.outputs().size(), out_shape->size());
+  imperative::CheckAndInferShape(g.get(), std::move(shape_inputs), true);


This may return false and the return value should be saved and server as the return value of ForeachShape.

reminisce · 2018-05-18T07:14:32Z

src/operator/nn/control_flow.cc

+    auto eid = idx.entry_id(input_nids[i], 0);
+    // If the input shape is none, we should update them.
+    if ((*in_shape)[i].ndim() == 0 || (*in_shape)[i].Size() == 0)
+      (*in_shape)[i] = shapes[eid];


To be more correct, use SHAPE_ASSIGN_CHECK. Same for the other places of assigning shapes to in_shape and out_shape.

reminisce · 2018-05-18T07:15:46Z

src/operator/nn/control_flow.cc

+  const auto& idx = g->indexed_graph();
+  CHECK_EQ(idx.input_nodes().size(), in_type->size());
+  CHECK_EQ(idx.outputs().size(), out_type->size());
+  imperative::CheckAndInferType(g.get(), std::move(dtype_inputs), true);


Save return value and return in the end.

reminisce · 2018-05-18T07:16:01Z

src/operator/nn/control_flow.cc

+  CHECK_EQ(input_nids.size(), in_type->size());
+  for (size_t i = 0; i < in_type->size(); i++) {
+    auto eid = idx.entry_id(input_nids[i], 0);
+    (*in_type)[i] = dtypes[eid];


TYPE_ASSIGN_CHECK.

reminisce · 2018-05-18T07:16:16Z

src/operator/nn/control_flow.cc

+  CHECK_EQ(idx.outputs().size(), out_attrs->size());
+  exec::DevMaskVector dev_masks(idx.num_nodes(), dev_mask);
+  StorageTypeVector storage_type_inputs = *in_attrs;
+  imperative::CheckAndInferStorageType(g.get(), std::move(dev_masks),


Save return value and return in the end.

reminisce · 2018-05-18T07:16:28Z

src/operator/nn/control_flow.cc

+  CHECK_EQ(input_nids.size(), in_attrs->size());
+  for (size_t i = 0; i < in_attrs->size(); i++) {
+    auto eid = idx.entry_id(input_nids[i], 0);
+    (*in_attrs)[i] = stypes[eid];


STORAGE_TYPE_ASSING_CHECK.

zheng-da · 2018-05-18T22:14:26Z

@piiswrong @eric-haibin-lin @reminisce @tqchen Could you please review this PR?

piiswrong · 2018-04-18T21:08:40Z

include/mxnet/imperative.h

-                    const nnvm::NodeAttrs& attrs,
-                    const std::vector<NDArray*>& inputs,
-                    const std::vector<NDArray*>& outputs);
+  static OpStatePtr Invoke(const Context& default_ctx,


use Imperative::Get()

piiswrong · 2018-04-18T21:09:41Z

python/mxnet/ndarray/contrib.py

@@ -96,3 +96,18 @@ def rand_zipfian(true_classes, num_sampled, range_max, ctx=None):
    expected_count_sampled = expected_prob_sampled * num_sampled
    return sampled_classes, expected_count_true, expected_count_sampled
 # pylint: enable=line-too-long
+
+def foreach(func, input, init_states, back_prop=False, name="foreach"):


back_prop -> Imperative::Get()->is_recording()
add OpContext::is_record at backend

piiswrong · 2018-04-18T21:20:42Z

python/mxnet/ndarray/contrib.py

+        ele = input[i]
+        outs, states = func(ele, states)
+        outs = _as_list(outs)
+        if (i == 0):


outputs.append(outs)
...

outputs = zip(*outputs)

[(a, b, c), (a2, b2, c2), ...] -> [(a, a, a, ...), (b, b, b, ...), ...]

piiswrong · 2018-04-18T21:35:08Z

src/operator/nn/control_flow.cc

+})
+.set_attr<nnvm::FListInputNames>("FListInputNames",
+    [](const NodeAttrs& attrs) {
+  return std::vector<std::string>{"fn", "data1", "data2"};


needs to be variable length

piiswrong · 2018-05-21T06:01:31Z

include/mxnet/c_api.h

+ * \param outs The input symbols of the graph.
+ * \param out_size the number of input symbols returned.
+ */
+MXNET_DLL int MXSymbolGetInputSymbols(SymbolHandle sym, SymbolHandle **outs,


We already have an ListInput api right?
should be **inputs?

here i need to get a list of input symbols instead of names. do you suggest merging these two APIs?

piiswrong · 2018-05-21T20:23:18Z

src/executor/attach_op_execs_pass.cc

@@ -134,15 +138,16 @@ class StatefulComputeExecutor : public StorageFallbackOpExecutor {
    return state_.get_var();
  }

-  explicit StatefulComputeExecutor(const OpStatePtr& state,
+  explicit StatefulComputeExecutor(const NodeAttrs& attrs, const OpStatePtr& state,


piiswrong · 2018-05-21T20:25:57Z

src/imperative/imperative_utils.h

@@ -379,7 +379,8 @@ inline void PushFCompute(const FCompute& fn,
                             &input_blobs, &output_blobs, &pre_temp_src, &pre_temp_dst,
                             &post_temp_src, &post_temp_dst, &in_temp_idx_map, mutate_idx);
      // setup context
-      OpContext opctx{is_train, rctx, engine::CallbackOnComplete(), requested};
+      bool need_grad = Imperative::Get()->is_recording();


need_grad shouldn't be get from the worker thread. It should be set outside similar to is_train

piiswrong · 2018-05-21T20:27:27Z

src/imperative/imperative_utils.h

-    if (exec_type == ExecType::kSync) {
+    // For operators with subgraphs, we need to invoke them in the main thread
+    // instead of the threaded engine.
+    if (!attrs.subgraphs.empty()) {


You wouldn't imperatively call an op with subgraphs right?

I think it can happen. For example, if we hybridize a block with control flow operators, the execution of these operators will happen here.

piiswrong · 2018-05-21T20:28:27Z

src/operator/nn/control_flow.cc

+  void Forward(std::vector<NDArray> cinputs,
+               const std::vector<OpReqType>& req,
+               std::vector<NDArray> coutputs, bool is_recording);
+  void Backward(int iter_no, std::vector<NDArray> ograds,


line break between args

piiswrong · 2018-05-21T20:29:55Z

src/operator/nn/control_flow.cc

+  std::unordered_map<std::string, std::vector<NDArray> > params;
+  CachedOpPtr op = std::make_shared<Imperative::CachedOp>(subgraph_sym, kwargs,
+                                                          arg_names, params);
+  // TODO(zhengda) we need to avoid shape inference and memory plan whenever the op is


why not allocate memory?

allocating memory, in general, is expensive. if we can avoid shape inference and memory allocation for each iteration, we should.

eric-haibin-lin · 2018-05-21T21:04:11Z

python/mxnet/symbol/contrib.py

+    return syms
+
+def foreach(func, data, init_states, name="foreach"):
+    """Run a for loop with user-defined computation over NDArrays on dimension 0.


NDArrays -> Symbols?

eric-haibin-lin · 2018-05-21T21:12:50Z

python/mxnet/ndarray/contrib.py

+
+    Parameters
+    ----------
+    func : a Python function.


Generic python function as an argument seems too broad. Since the interface for func is well defined, do we want to restrict it to a well-defined python class? For example,

class ForeachBody(object): def forward(data, states): raise NotImplementedError() def __call__(data, states): """ data: NDArray or list of NDArrays states: NDArray or list of NDArrays ... """ check_input(data, states) return self.forward(data,states) def foreach(body, data, state) Parameters ---------- func : a ForeachBody.

Then you don't have to do check_input inside contrib.foreach

this is to follow the interface of TensorFlow. https://www.tensorflow.org/api_docs/python/tf/while_loop
Using class does make API more well defined, but it requires users to write more code to define it. I don't know what is the best way.
@piiswrong what's your opinion?

eric-haibin-lin · 2018-05-21T21:21:12Z

python/mxnet/symbol/contrib.py

+from ..base import _LIB, c_array, check_call
+from ..base import SymbolHandle, _as_list
+from ..attribute import AttrScope
+
 __all__ = ["rand_zipfian"]


Add foreach to __all__ ?

what is this for?

Otherwise from contrib import * won't include the foreach function

eric-haibin-lin · 2018-05-21T21:27:11Z

python/mxnet/symbol/contrib.py

+    # the python function, we need to prune the computation graph constructed from
+    # the function. One way of doing it is to mark the nodes in the computation graph
+    # with AttrScope and prune the nodes without the special attribute.
+    with AttrScope(subgraph_name=name):


What's the alternative?

alternative of what?

eric-haibin-lin · 2018-05-21T21:38:02Z

src/executor/graph_executor.cc

@@ -1537,6 +1555,9 @@ GraphExecutor::CachedSegOpr GraphExecutor::CreateCachedSegOpr(size_t topo_start,
    OpNode& op_node = op_nodes_[nid];
    if (op_node.skip_exec_node) continue;
    if (inode.source->is_variable()) continue;
+    // We shouldn't add control flow operators to a segment.
+    // We can't execute these operators in the engine.
+    if (op_node.exec->HasSubgraph()) continue;


Why not return ret instead of continue?

using return ret means breaking the graph into two pieces?

eric-haibin-lin · 2018-05-21T21:43:23Z

src/imperative/imperative_utils.h

 #if MXNET_USE_MKLDNN == 1
      InvalidateOutputs(outputs, req);
 #endif
      fcompute_ex(state, opctx, inputs, req, outputs);
-      if (ctx.dev_mask() == gpu::kDevMask && exec_type == ExecType::kSync) {


Good catch. I think we need to check
bool is_gpu = rctx.get_ctx().dev_mask() == gpu::kDevMask;

eric-haibin-lin · 2018-05-21T21:43:38Z

src/imperative/imperative_utils.h

@@ -505,12 +515,16 @@ inline void PushOperator(const OpStatePtr& state,
        fcompute(state, opctx, input_blobs, tmp_req, output_blobs);
        // post-fcompute fallback, cast to original storage type, if necessary
        CastNonDefaultStorage(post_temp_src, post_temp_dst, opctx, is_gpu);
-        if (is_gpu && exec_type == ExecType::kSync) {
+        if (is_gpu && exec_type == ExecType::kSync


Why is && rctx.get_stream<gpu>() required?

because subgraph operators don't run in the threaded engine and don't have gpu stream.

eric-haibin-lin · 2018-05-21T21:48:47Z

src/operator/nn/control_flow.cc

+  if (len % 2 == 1) {
+    for (size_t i = 1; i < subg_outputs1.size(); i++) {
+      subg_outputs1[i] = outputs[i];
+      subg_outputs2[i] = NDArray(outputs[i].shape(), outputs[i].ctx(), true,


This assumes all NDArrays are dense?

eric-haibin-lin · 2018-05-21T22:36:04Z

src/operator/nn/control_flow.cc

+    ograds[i] = inputs[i];
+  std::vector<OpReqType> iter_req(req.size());
+  for (auto r : req)
+    CHECK_NE(r, kWriteInplace);


Is req guaranteed not to be equal to kWriteInplace?

eric-haibin-lin · 2018-05-21T22:39:28Z

src/operator/nn/subgraph_op_common.h

+ * under the License.
+ */
+
+#ifndef MXNET_OPERATOR_NN_SUBGRAPH_OP_COMMON_H_


Why is subgraph op inside nn/ folder? Isn't it more general?

you are right. i probably should move control flow op out as well.

junrushao · 2018-06-08T16:54:24Z

src/operator/control_flow.cc

+
+struct ForeachParam : public dmlc::Parameter<ForeachParam> {
+  int num_args;
+  int dim;


Is the field "int dim" used anywhere?

it's not. i can remove it for now.

junrushao · 2018-06-08T16:57:45Z

src/operator/control_flow.cc

+                                const std::vector<NDArray>& outputs) {
+  ForeachState &state = state_ptr.get_state<ForeachState>();
+  const ForeachParam& params = state.params;
+  size_t iter_dim = 0;


would you mind adding a "constexpr" specifier?

junrushao · 2018-06-08T16:58:18Z

src/operator/control_flow.cc

+  DMLC_DECLARE_PARAMETER(ForeachParam) {
+    DMLC_DECLARE_FIELD(num_args).set_lower_bound(1)
+    .describe("Number of inputs.");
+    DMLC_DECLARE_FIELD(dim).set_default(1)


Is this default to 1 or 0?

junrushao · 2018-06-08T16:58:53Z

src/operator/subgraph_op_common.cc

+  std::vector<std::pair<std::string, std::string> > kwargs;
+  kwargs.push_back(std::pair<std::string, std::string>("inline_limit", "0"));
+  // Get input names.
+  const auto& idx = subgraph.indexed_graph();


Seems the "idx" here is unused

junrushao · 2018-06-08T20:07:47Z

python/mxnet/symbol/contrib.py

+            is_NDArray_or_list = isinstance(inputs, in_type)
+        assert is_NDArray_or_list, msg
+
+    check_data(data, symbol.Symbol, "data should be an NDArray or a list of NDArrays")


Should use symbol instead of ndarray in the error message

junrushao · 2018-06-08T22:20:23Z

python/mxnet/ndarray/contrib.py

+            "init_states should be an NDArray or a list of NDArrays")
+
+    not_data_list = isinstance(data, ndarray.NDArray)
+    not_state_list = isinstance(init_states, ndarray.NDArray)


This boolean variable is not used, you probably should check it during the loop

tqchen · 2018-06-14T23:59:20Z

One suggestion, since this ops is common known as scan, why now just use the common name instead of inventing a new API name?

zheng-da · 2018-06-15T20:21:05Z

@tqchen Originally, I consider this as a control flow operator, so I use foreach because a lot of languages use foreach as a keyword for this. But DL frameworks consider this operator as a high-order function and call it scan. It makes sense to follow the convention of the DL frameworks, although the definition of this operator is a little different from the one in TensorFlow (foreach here splits outputs of an iteration into outputs of the loop and loop variables).

eric-haibin-lin · 2018-06-15T20:52:08Z

python/mxnet/symbol/contrib.py

+from ..base import _LIB, c_array, check_call
+from ..base import SymbolHandle, _as_list
+from ..attribute import AttrScope
+
 __all__ = ["rand_zipfian"]


Otherwise from contrib import * won't include the foreach function

eric-haibin-lin · 2018-06-15T20:52:46Z

python/mxnet/symbol/contrib.py

@@ -91,3 +98,154 @@ def rand_zipfian(true_classes, num_sampled, range_max):
    expected_prob_sampled = ((sampled_cls_fp64 + 2.0) / (sampled_cls_fp64 + 1.0)).log() / log_range
    expected_count_sampled = expected_prob_sampled * num_sampled
    return sampled_classes, expected_count_true, expected_count_sampled
+
+def _get_graph_inputs(subg):
+    num_handles = ctypes.c_int(1000)


eric-haibin-lin · 2018-06-15T20:54:48Z

src/operator/subgraph_op_common.h

+ * This contains the states for running a loop and provides methods
+ * of running the subgraph computation for an iteration.
+ */
+class LoopState {


Is this state going to be shared by while loop or just foreach?

it's supposed to be shared by while loop.

eric-haibin-lin · 2018-06-15T20:56:52Z

src/operator/subgraph_op_common.cc

+}
+
+void LoopState::Backward(int iter_no,
+                         std::vector<NDArray> ograds,


const reference?

The reason it's not const reference is that we need a copy of the NDArray vector anyway. we need to pass their pointers to cached op, which requests pointers instead of const pointers.

eric-haibin-lin · 2018-06-15T20:58:19Z

src/operator/subgraph_op_common.cc

+  // TODO(zhengda) we need to avoid shape inference and memory plan whenever the op is
+  // called. Currently, CachedOp allocates memory each time Forward is called.
+  // I need to fix this once the PR for static memory allocation in CachedOp is
+  // merged. https://github.com/apache/incubator-mxnet/pull/10817


PR for static mem was merged

eric-haibin-lin · 2018-06-15T21:18:04Z

src/operator/subgraph_op_common.cc

+  const auto& idx = g.indexed_graph();
+  CHECK_EQ(idx.input_nodes().size(), in_type->size());
+  CHECK_EQ(idx.outputs().size(), out_type->size());
+  imperative::CheckAndInferType(&g, std::move(dtype_inputs), true);


What if some out_type is known and we want to perform mutual inference based on outputs for elemwise ops?

eric-haibin-lin · 2018-06-15T21:22:52Z

src/operator/control_flow.cc

+    CHECK_EQ(len, outputs[i].shape()[iter_dim]);
+  for (const auto &arr : outputs)
+    CHECK_EQ(arr.storage_type(), kDefaultStorage)
+        << "The for operator doesn't support the sparse format";


Curious: Is there anything special to handle for sparse nd in foreach??

because I created NDArrays below. To handle sparse arrays, I need to create sparse arrays explicitly. I'm not sure if foreach needs to handle sparse arrays in general. So this version will just handle dense arrays first.

eric-haibin-lin · 2018-06-15T21:26:54Z

src/operator/control_flow.cc

+    ograds[i] = inputs[i];
+  std::vector<OpReqType> iter_req(req.size());
+  for (auto r : req)
+    CHECK_NE(r, kWriteInplace);


Is this guaranteed? If plan memory introduced writeinplace in the backward, how does an user work around that?

To enable WriteInplace, an operator needs to enable FInplaceOption, right? foreach doesn't have the attribute for either forward or backward.

eric-haibin-lin · 2018-06-15T21:28:16Z

src/imperative/imperative_utils.h

          rctx.get_stream<gpu>()->Wait();
        }
      };

-    if (exec_type == ExecType::kSync) {
+    if (!attrs.subgraphs.empty()) {


what about setting exec_type for Foreach op to kSubgraph instead of checking attrs.subgraphs?

eric-haibin-lin · 2018-06-15T21:28:58Z

python/mxnet/ndarray/contrib.py

@@ -96,3 +98,97 @@ def rand_zipfian(true_classes, num_sampled, range_max, ctx=None):
    expected_count_sampled = expected_prob_sampled * num_sampled
    return sampled_classes, expected_count_true, expected_count_sampled
 # pylint: enable=line-too-long
+
+def foreach(body, data, init_states):


You probably want to also update ndarray/contrib.md and symbol/contrib.md

When two node entries refer to the same output of a node, we should create only one var node for these two node entries.

We can't get a list of variable names from a hashtable. The order can't be guaranteed. Python2 and Python3 output different orders.

zheng-da requested a review from szha as a code owner April 7, 2018 01:19

zheng-da mentioned this pull request Apr 7, 2018

Pass a graph symbol to an operator dmlc/nnvm#430

Closed

zheng-da force-pushed the foreach branch from 9e964b0 to 7b0bcd1 Compare April 9, 2018 21:16

zheng-da force-pushed the foreach branch 2 times, most recently from e75405a to 1fa88a7 Compare May 15, 2018 01:37

eric-haibin-lin self-assigned this May 15, 2018

reminisce reviewed May 17, 2018

View reviewed changes

zheng-da force-pushed the foreach branch from 1fa88a7 to 9a4a229 Compare May 18, 2018 06:20

reminisce reviewed May 18, 2018

View reviewed changes

zheng-da changed the title ~~[WIP] Add Foreach~~ [MXNET-432] Add Foreach May 18, 2018

zhanghang1989 approved these changes May 19, 2018

View reviewed changes

zheng-da force-pushed the foreach branch from b7339c2 to 28ba842 Compare May 19, 2018 00:52

piiswrong suggested changes May 21, 2018

View reviewed changes

eric-haibin-lin reviewed May 21, 2018

View reviewed changes

zheng-da mentioned this pull request May 25, 2018

Cut subgraph #11059

Closed

7 tasks

zheng-da force-pushed the foreach branch from b849caf to 9987b97 Compare June 4, 2018 23:00

junrushao reviewed Jun 8, 2018

View reviewed changes

eric-haibin-lin removed their assignment Jun 13, 2018

zheng-da force-pushed the foreach branch from 13c2994 to a94eca3 Compare June 14, 2018 22:42

zheng-da force-pushed the foreach branch from a94eca3 to fab1156 Compare June 15, 2018 17:51

eric-haibin-lin reviewed Jun 15, 2018

View reviewed changes

This was referenced Jun 16, 2018

Static alloc for hybridblock #11313

Merged

Revert "Static alloc for hybridblock" #11318

Merged

zheng-da added 25 commits July 2, 2018 18:20

update comments.

25e15a0

fix scala.

ff4eea0

fix test.

b8aa62a

fix attribute name.

64e4ff6

move benchmark.

d243c12

fix the mapping of operator inputs/outputs and subgraph inputs/outputs.

3afc4d4

add tests for dtype/shape inference.

62901fe

reorganize tests.

14b8fb9

fix a bug of cutting NodeEntry.

f7d7f17

When two node entries refer to the same output of a node, we should create only one var node for these two node entries.

fix lint error.

7d012d9

handle the case that outputs are inputs.

b83253d

handle the case that inputs aren't used.

275bbf1

handle the case without output data.

0e6df9a

fix a bug in foreach backward.

dfadc8d

fix a bug when there isn't output data.

5e9cf5f

Fix lint error.

696f53c

test diff Gluon RNN cells.

094977d

test all symbol RNN cells.

7b016ae

adjust the test precision.

9609ce8

Fix a bug in getting a list of variable names.

fa8abbd

We can't get a list of variable names from a hashtable. The order can't be guaranteed. Python2 and Python3 output different orders.

fix lint error.

8e74d80

Test 1D array.

2439105

fix a bug when subgraph inputs and outputs share NDArray.

53cdbfa

fix.

9bff317

fix

d3687ef

zheng-da force-pushed the foreach branch from 9086154 to d3687ef Compare July 2, 2018 18:20

zheng-da requested a review from nswamy as a code owner July 2, 2018 18:20

add comments.

392a7e4

zheng-da mentioned this pull request Jul 2, 2018

[MXNET-432] Add Foreach #11531

Merged

5 tasks

zheng-da closed this Jul 2, 2018

[MXNET-432] Add Foreach #10451

[MXNET-432] Add Foreach #10451

Conversation

zheng-da commented Apr 7, 2018 • edited Loading

Description

Checklist

Essentials

marcoabreu commented Apr 7, 2018

zheng-da commented Apr 7, 2018

reminisce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zheng-da commented May 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zheng-da May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented Jun 14, 2018

zheng-da commented Jun 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zheng-da commented Apr 7, 2018 •

edited

Loading

zheng-da May 21, 2018 •

edited

Loading