junrushao1994 commented on a change in pull request #14192: [MXNET-1324] Add 
NaiveRunGraph to imperative utils
URL: https://github.com/apache/incubator-mxnet/pull/14192#discussion_r293638551
 
 

 ##########
 File path: src/imperative/cached_op.cc
 ##########
 @@ -262,6 +262,35 @@ std::vector<nnvm::NodeEntry> CachedOp::Gradient(
   return ret;
 }
 
+bool CachedOp::CheckDynamicShapeExists(const Context& default_ctx,
+                                       const std::vector<NDArray*>& inputs,
+                                       bool erase_result) {
+  using namespace nnvm;
+  using namespace imperative;
+  CHECK_EQ(inputs.size(), num_inputs());
+
+  auto state_ptr = GetCachedOpState(default_ctx);
+  auto& state = state_ptr.get_state<CachedOpState>();
+
+  nnvm::Graph& g = state.info.fwd_graph;
+  ShapeVector shape_inputs;
+  shape_inputs.reserve(inputs.size());
+  for (auto input : inputs) {
+    shape_inputs.emplace_back(input->shape());
+  }
+  // We leverage the shape inference pass to detect whether dynamic shape 
exists.
+  // If so, the pass will fail with `contain_dynamic_shape = true`,
+  // This method is only called once, so the overhead is negligible.
 
 Review comment:
   If infer shape fails, it means there is probably an op of dynamic shape 
(np.unique, boolean mask).
   
   MXNet didn't support this kind of op before, because of the limitation of 
our system. Because of these lines, we now can support it in Gluon blocks.
   
   I would suggest to clearly mention the behavior in our docs that if infer 
shape fails, the code will go to the slow path (naive run graph, etc).

----------------------------------------------------------------
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]


With regards,
Apache Git Services

Reply via email to