ZhennanQin commented on a change in pull request #15886: Graph Partition API
URL: https://github.com/apache/incubator-mxnet/pull/15886#discussion_r318845381
 
 

 ##########
 File path: src/c_api/c_api_symbolic.cc
 ##########
 @@ -1199,3 +1200,73 @@ int MXShallowCopySymbol(SymbolHandle src, SymbolHandle* 
out) {
   *out = out_sym;
   API_END_HANDLE_ERROR(delete out_sym);
 }
+
+int MXOptimizeForBackend(SymbolHandle sym_handle,
+                         const char* backend_name,
+                         SymbolHandle* ret_sym_handle,
+                         const mx_uint len,
+                         NDArrayHandle* in_args_handle,
+                         const mx_uint num_options,
+                         const char** keys,
+                         const char** vals) {
+  nnvm::Symbol *s = new nnvm::Symbol();
+  API_BEGIN();
+  nnvm::Symbol *sym = static_cast<nnvm::Symbol *>(sym_handle);
+  *s = sym->Copy();
+  nnvm::Graph g = Symbol2Graph(*s);
+  if (len) {
+    NDArray **in_args_ptr = reinterpret_cast<NDArray**>(in_args_handle);
+    Context default_ctx = in_args_ptr[0]->ctx();
 
 Review comment:
   > Instead we'll pull the context from where the args reside.
   
   args are normally loaded by
   ```
   sym, args, aux = mx.model.load_checkpoint('resnet-50', 0)
   ```
   No contexts are specified. That's why we need to pass the expected contexts 
during bind. The contexts from args doesn't mean anything.
   
   >We expect that users could call this more than once for the same model. For 
example, users could first optimize for EIA (that would group supported nodes 
into subgraphs) and then optimize for MKLDNN for the remaining nodes that were 
not partitioned into subgraphs.
   
   That's the comments you provided earlier. So I thought you're working to 
support multi-context execution. If not, then you mean we only support running 
multiple `optimize_for` for same contexts? Even for that, we need to pass 
correct contexts for `optimize_for`, not the gathered one from args.
   
   Now again, the correct contexts are important for `InferStorageTypes` for 
MKLDNN backend, so we need to pass correct context(same as bind) to 
`optimize_for`. If you don't want to introduce `ctx` for `optimize_for` API, 
then I suggest to query the expected context from backend property, and use it 
to pass `InferStorageTypes`. Or, you can figure out other solution to solve 
this, I just want to ensure that subgraph partition can get same 
`InferStorageTypes` results as bind.

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