[GitHub] zheng-da commented on a change in pull request #9977: Cpu lstm inference

2018-03-07 Thread GitBox
zheng-da commented on a change in pull request #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#discussion_r173038522
 
 

 ##
 File path: src/operator/rnn-inl.h
 ##
 @@ -287,8 +507,9 @@ class RNNProp : public OperatorProperty {
 const std::vector &out_grad,
 const std::vector &in_data,
 const std::vector &out_data) const override {
-std::vector dep = {in_data[rnn_enum::kData], 
in_data[rnn_enum::kParams],
-in_data[rnn_enum::kState], out_data[rnn_enum::kOut], 
out_grad[rnn_enum::kOut]};
+std::vector dep = {in_data[rnn_enum::kData],
+  in_data[rnn_enum::kParams], in_data[rnn_enum::kState],
+  out_data[rnn_enum::kOut], out_grad[rnn_enum::kOut]};
 
 Review comment:
   the coding style in mxnet allows up to 100 char per line.
   so the original code is fine.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #9977: Cpu lstm inference

2018-03-07 Thread GitBox
zheng-da commented on a change in pull request #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#discussion_r172948107
 
 

 ##
 File path: src/operator/rnn-inl.h
 ##
 @@ -144,15 +149,224 @@ class RNNOp : public Operator {
 const std::vector &req,
 const std::vector &in_grad,
 const std::vector &aux_args) {
-using namespace mshadow;
-using namespace mshadow::expr;
 // TODO(sbodenstein): add MShadow implementation
   }
 
  private:
   RNNParam param_;
 };  // class RNNOp
 
+template
+class RNNOp : public Operator {
+ public:
+  explicit RNNOp(RNNParam param) {
+this->param_ = param;
+// RNN Mode
+switch (param_.mode) {
+  case rnn_enum::kLstm:
+break;
+  default:
+LOG(FATAL) << "only LSTM is implmented on CPU";
+}
+if (param_.mode == rnn_enum::kLstm)
+  param_.lstm_q_ = true;
+else
+  param_.lstm_q_ = false;
 
 Review comment:
   it seems this check can be merged to the switch case statement above.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #9977: Cpu lstm inference

2018-03-07 Thread GitBox
zheng-da commented on a change in pull request #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#discussion_r172710204
 
 

 ##
 File path: src/operator/rnn-inl.h
 ##
 @@ -78,10 +82,12 @@ inline int rnn_param_size(int layerNum,
   int size = rnn_single_param_size(inputSize, hiddenSize, mode);
   // get size of remaining layers
   if (bidirectional) {
-size += (layerNum - 1) * rnn_single_param_size(2 * hiddenSize, hiddenSize, 
mode);
+size += (layerNum - 1) * rnn_single_param_size(2 * hiddenSize,
+hiddenSize, mode);
 size *= 2;
   } else {
-size += (layerNum - 1) * rnn_single_param_size(hiddenSize, hiddenSize, 
mode);
+size += (layerNum - 1) * rnn_single_param_size(hiddenSize, hiddenSize,
+mode);
 
 Review comment:
   you are just reformatting the code here?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #9977: Cpu lstm inference

2018-03-07 Thread GitBox
zheng-da commented on a change in pull request #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#discussion_r172944904
 
 

 ##
 File path: src/operator/rnn-inl.h
 ##
 @@ -287,8 +507,9 @@ class RNNProp : public OperatorProperty {
 const std::vector &out_grad,
 const std::vector &in_data,
 const std::vector &out_data) const override {
-std::vector dep = {in_data[rnn_enum::kData], 
in_data[rnn_enum::kParams],
-in_data[rnn_enum::kState], out_data[rnn_enum::kOut], 
out_grad[rnn_enum::kOut]};
+std::vector dep = {in_data[rnn_enum::kData],
+  in_data[rnn_enum::kParams], in_data[rnn_enum::kState],
+  out_data[rnn_enum::kOut], out_grad[rnn_enum::kOut]};
 
 Review comment:
   i'm not sure why you want to change the code in this function. it seems you 
just reorganize the code a little bit.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] zheng-da commented on a change in pull request #9977: Cpu lstm inference

2018-03-07 Thread GitBox
zheng-da commented on a change in pull request #9977: Cpu lstm inference
URL: https://github.com/apache/incubator-mxnet/pull/9977#discussion_r172712476
 
 

 ##
 File path: src/operator/rnn-inl.h
 ##
 @@ -144,15 +149,224 @@ class RNNOp : public Operator {
 const std::vector &req,
 const std::vector &in_grad,
 const std::vector &aux_args) {
-using namespace mshadow;
-using namespace mshadow::expr;
 // TODO(sbodenstein): add MShadow implementation
   }
 
  private:
   RNNParam param_;
 };  // class RNNOp
 
+template
+class RNNOp : public Operator {
+ public:
+  explicit RNNOp(RNNParam param) {
+this->param_ = param;
+// RNN Mode
+switch (param_.mode) {
+  case rnn_enum::kLstm:
+break;
+  default:
+LOG(FATAL) << "only LSTM is implmented on CPU";
+}
+if (param_.mode == rnn_enum::kLstm)
+  param_.lstm_q_ = true;
+else
+  param_.lstm_q_ = false;
+  }
+
+  virtual void Forward(const OpContext &ctx,
+   const std::vector &in_data,
+   const std::vector &req,
+   const std::vector &out_data,
+   const std::vector &aux_args) {
+// Layout TNC
+
+size_t in_expected = param_.lstm_q_ ? 4 : 3;
+size_t out_expected = param_.lstm_q_ ? 3 : 2;
+
+if (!param_.state_outputs)
+  LOG(FATAL) << "no state outputs is currently not supported for cpu.";
+
+CHECK_EQ(req[rnn_enum::kOut], kWriteTo);
+CHECK_EQ(in_data.size(), in_expected);
+CHECK_EQ(out_data.size(), out_expected);
+
+mshadow::Stream *s = ctx.get_stream();
+// get input + output tensors
+// w layout i2h_w, h2h_w, i2h_b, h2h_b
+Tensor x =
+in_data[rnn_enum::kData].get(s);  // TNC
+Tensor w = in_data[rnn_enum::kParams].get(s);
+Tensor hx =
+in_data[rnn_enum::kState].get(s);  // LNC
+Tensor y =
+out_data[rnn_enum::kOut].get(s);  // TNC
+int64_t seq_len = x.shape_[0];
+int64_t num_layers = hx.shape_[0];
+int64_t batch_size = x.shape_[1];
+int64_t h_channel = hx.shape_[2];
+int64_t in_channel = x.shape_[2];
+Tensor x_flatten = in_data[rnn_enum::kData]
+  .get_with_shape(
+  mshadow::Shape2(seq_len * batch_size, in_channel), s);  // (T*N)C
+Tensor y_flatten = out_data[rnn_enum::kOut]
+  .get_with_shape(
+  mshadow::Shape2(
+  y.shape_[0] * y.shape_[1], y.shape_[2]), s);  // (T*N)C
+
+CHECK_EQ(x.CheckContiguous(), true);
+CHECK_EQ(w.CheckContiguous(), true);
+CHECK_EQ(hx.CheckContiguous(), true);
+CHECK_EQ(y.CheckContiguous(), true);
+
+if (ctx.is_train)
+  LOG(FATAL) << "only inference mode is available for cpu at the moment.";
 
 Review comment:
   you can do CHECK(!ctx.is_train) << "..."


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services