[GitHub] [incubator-mxnet] waytrue17 commented on a change in pull request #18690: Optimize graph execution in presence of dynamic shape op

[GitBox <gi...@apache.org>]

waytrue17 commented on a change in pull request #18690:
URL: https://github.com/apache/incubator-mxnet/pull/18690#discussion_r516988225



##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -329,7 +368,21 @@ void PreSelectSubgraphNodes(const nnvm::Graph& g, SubgraphSelectorV2Ptr subgraph
     }
     ++count;
   }
-  if (!success) {
+  if (success) {
+    // CachedOp requires external input during forward pass
+    // check subgraph input. If none, reject the first op (in top order) from the subgraph
+    // to make sure CachedOp gets external input.
+    // this feature can be switched off by setting ensure_CachedOp_input to false
+    const SubgraphPropertyPtr& subg_prop = g.GetAttr<SubgraphPropertyPtr>("subgraph_property");
+    if (subg_prop->HasAttr("ensure_CachedOp_input")
+        && subg_prop->GetAttr<bool>("ensure_CachedOp_input")) {
+      if (subgraph_nodes->size() > 0 && !HasInputEntries(g, simple_nodes, *subgraph_nodes)) {
+        // relabel the node to -1
+        (*subgraph_nodes)[0]->label = -1;
+        subgraph_nodes->erase(subgraph_nodes->begin());

Review comment:
       here we wanted to check if cachedOp has external input, which requires both the entire graph and the subgraph. 
   Seems it’s not trivial to move this functionality to `filter` as it is not easy to pass the info to there. 




----------------------------------------------------------------
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:
users@infra.apache.org