You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/02/19 01:37:35 UTC
[GitHub] [incubator-mxnet] samskalicky opened a new pull request #17623:
Dynamic subgraph compile support
samskalicky opened a new pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623
## Description ##
This PR adds support for passing the ndarrays from the existing `optimize_for` API down to the `acceptSubgraph` function in an external library. There are no user-facing API changes.
- Modifies the subgraph library example to optionally require args to be provided
- Adds a new test to partition operators that directly consume params
- Adds annotation on subgraph inputs for the name of the original param so that inputs can be mapped
## Design ##
In #15886 the `optimize_for` API was added to give users an easy API to use to partition their models. The `args` argument took the params to the model to use for shape/type inference. But the ndarray values were never used. In this PR, we pass the ndarray data values to the backend library. There is no change in the API for `optimize_for`:
```
sym = sym.optimize_for('default', args=args, ctx=mx.cpu())
```
On the backend library side, the `acceptSubgraph` API has an addition argument for `args` that is a map of named MXTensors. These are the same `args` that the user passed into the front-end `optimize_for` API.
```
MXReturnValue acceptSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
std::unordered_map<std::string, std::string>& attrs) {
std::unordered_map<std::string, std::string>& attrs,
std::map<std::string, MXTensor>& args);
```
This additional map of args will allow backends to compile subgraphs and use param/weight values during compilation. For example, this will enable TensorRT to be implemented as a backend library and eliminate the `init_tensorrt_params` API that is needed to provide the params to the TensorRT backend. It will also enable compiling subgraphs with TVM and other compile-specific backends.
## 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](https://issues.apache.org/jira/projects/MXNET/issues) 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 https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
- [ ] To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
### Changes ###
- [ ] Feature1, tests, (and when applicable, API doc)
- [ ] Feature2, tests, (and when applicable, API doc)
## Comments ##
- If this change is a backward incompatible change, why must this change be made.
- Interesting edge cases to note here
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-588096257
@mxnet-label-bot add [pr-awaiting-review]
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393486933
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
+ ss << shape.substr(1, shape.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
}
- // free memory used by custom op to allocate attributes
- call_free_(attr_vals);
- call_free_(attr_keys);
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = n->attrs.dict["__dtype__"];
+ ss << dtype.substr(1, dtype.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
+ }
+ // set user specified attributes
+ for (auto attr : user_attrs)
+ n->attrs.dict[attr.first] = attr.second;
+
return n;
} else {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ for (size_t i = 0; i < input_entries->size(); ++i) {
+ nnvm::NodeEntry *e = input_entries->at(i);
+ nnvm::NodeEntry& orig = orig_input_entries->at(i);
+
+ // set attribute for subgraph input to indicate if it is from an arg/param to model
+ if (orig.node->is_variable()) {
+ // get name of original output entry
+ nnvm::Symbol sym;
+ sym.outputs.push_back(orig);
+ const auto output_names = sym.ListOutputNames();
+ CHECK_EQ(output_names.size(), 1U);
+ const std::string& var_name = output_names[0];
+
+ e->node->attrs.dict["isArg"] = "True";
+ e->node->attrs.dict["argName"] = var_name;
+ } else {
+ e->node->attrs.dict["isArg"] = "False";
+ }
+
+ // pass down other attributes if available
+ if (orig.node->attrs.dict.count("__dtype__") > 0) {
+ // get dtype string from other node
+ std::string& dtype = orig.node->attrs.dict["__dtype__"];
+ int idx = 0;
+ // find the beginning of the output dtype for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = dtype.find("[", idx+1);
Review comment:
fixed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384248544
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
no, auxName is not necessary.` argName` will be set for all "variables" which includes both args and aux.
for an arg param, isArg = True && argName = 'bob'
for an aux param, isArg = True && isAux = true && argName = 'bob'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r392534373
##########
File path: src/c_api/c_api.cc
##########
@@ -122,14 +122,28 @@ void CustomFComputeDispatcher(const std::string op_name,
// convert inputs/outpus NDArray to C types to be passed to lib_api.h
for (size_t i = 0; i < inputs.size(); i++) {
- in_data.push_back(inputs[i].data().dptr_);
- in_shapes.push_back(inputs[i].shape().data());
- in_dims.push_back(inputs[i].shape().ndim());
- in_types.push_back(inputs[i].dtype());
- in_verIDs.push_back(inputs[i].version());
- const char* ctx_str = inputs[i].ctx().dev_mask() == Context::kCPU ? "cpu" : "gpu";
- in_dev_type.push_back(ctx_str);
- in_dev_id.push_back(inputs[i].ctx().real_dev_id());
+ const NDArray& in_nd = inputs[i];
+ // reorder data if in MKLDNN format
+ if (in_nd.IsMKLDNNData()) {
+ const NDArray& tmp_nd = in_nd.Reorder2Default();
Review comment:
Can we just keep `in_nd.Reorder2Default()` inside if block, the rest looks same for if and else block.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r392556534
##########
File path: src/c_api/c_api.cc
##########
@@ -122,14 +122,28 @@ void CustomFComputeDispatcher(const std::string op_name,
// convert inputs/outpus NDArray to C types to be passed to lib_api.h
for (size_t i = 0; i < inputs.size(); i++) {
- in_data.push_back(inputs[i].data().dptr_);
- in_shapes.push_back(inputs[i].shape().data());
- in_dims.push_back(inputs[i].shape().ndim());
- in_types.push_back(inputs[i].dtype());
- in_verIDs.push_back(inputs[i].version());
- const char* ctx_str = inputs[i].ctx().dev_mask() == Context::kCPU ? "cpu" : "gpu";
- in_dev_type.push_back(ctx_str);
- in_dev_id.push_back(inputs[i].ctx().real_dev_id());
+ const NDArray& in_nd = inputs[i];
+ // reorder data if in MKLDNN format
+ if (in_nd.IsMKLDNNData()) {
+ const NDArray& tmp_nd = in_nd.Reorder2Default();
Review comment:
`Reorder2Default` returns a copy:
https://github.com/apache/incubator-mxnet/blob/4dddb087293965af7e7af8d0eaead60a9051f0fc/src/ndarray/ndarray.cc#L597
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393486892
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
Review comment:
fixed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384225231
##########
File path: example/extensions/lib_subgraph/test_subgraph.py
##########
@@ -88,3 +91,54 @@
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
print(out4)
+# Gluon Hybridize partitioning with shapes/types without inference
+print('-------------------------------')
+print('Testing Gluon Hybridize partitioning with shapes/types without inference')
+inputs = [a,b]
+sym_block2 = nn.SymbolBlock(sym, inputs)
+sym_block2.initialize()
+sym_block2.optimize_for(mx.nd.ones((3,2)), mx.nd.ones((3,2)), backend='myProp')
+sym_block2.export('partitioned')
+
+
+###############################################
+# Test with subgraph directly consuming params
+###############################################
+# example model, ops to be partitioned have args
+d2 = mx.sym.exp(a)
Review comment:
I don't understand the goal of this test.
The point of optimize_for() is to export the partitioned graph without running inference.
Therefore, in the real world use case, the user will only run inference on the imported partition.
The test should re-import the partitioned graph and test that right ?
Testing the sym directly tells us nothing about the success of the export/import.
NOTE : There may still be value to testing the sym directly but that is secondary to the intended use model
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388630569
##########
File path: python/mxnet/symbol/symbol.py
##########
@@ -1475,13 +1482,19 @@ def optimize_for(self, backend, args=None, ctx=None, **kwargs):
out = SymbolHandle()
assert isinstance(backend, str)
- if args is None:
+ if args is None or len(args) == 0:
args = []
args_handle = c_array(NDArrayHandle, [])
else:
- listed_arguments = self.list_arguments()
- args_handle, args = self._get_ndarray_inputs('args', args, listed_arguments, False)
+ args_handle, args = self._get_ndarray_inputs('args', args,
+ self.list_arguments(), False)
+ if aux is None or len(aux) == 0:
+ aux = []
Review comment:
do we need this line?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384226462
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
sanity check .... are we attaching "auxName" to auxes ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384693188
##########
File path: example/extensions/lib_subgraph/test_subgraph.py
##########
@@ -88,3 +91,54 @@
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
print(out4)
+# Gluon Hybridize partitioning with shapes/types without inference
+print('-------------------------------')
+print('Testing Gluon Hybridize partitioning with shapes/types without inference')
+inputs = [a,b]
+sym_block2 = nn.SymbolBlock(sym, inputs)
+sym_block2.initialize()
+sym_block2.optimize_for(mx.nd.ones((3,2)), mx.nd.ones((3,2)), backend='myProp')
+sym_block2.export('partitioned')
+
+
+###############################################
+# Test with subgraph directly consuming params
+###############################################
+# example model, ops to be partitioned have args
+d2 = mx.sym.exp(a)
Review comment:
agreed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388529468
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
Review comment:
Ya i think i just copied from hybridize's doc string. I'll update this so its more clear what the function does and refer to hybridize. i'll add something like we put in the README.md:
> Immediately partitions a HybridBlock using the specified backend. Combines the work done in the `hybridize` API with part of the work done in the forward pass without calling the CachedOp. Can be used in place of `hybridize`, afterwards `export` can be called or inference can be run.
What do you think?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-599918951
@mseth10 thanks! lmk what you think of the latest commit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388641653
##########
File path: include/mxnet/lib_api.h
##########
@@ -407,6 +412,8 @@ class OpResource {
*/
/*! \brief Macro to help passing serialized subgraph through attribute dict */
#define SUBGRAPH_SYM_JSON "subgraph_sym_json"
+#define MX_DTYPE "__dtype__"
+#define MX_SHAPE "__shape__"
Review comment:
changed to:
```
#define MX_STR_SUBGRAPH_SYM_JSON "subgraph_sym_json"
#define MX_STR_DTYPE "__dtype__"
#define MX_STR_SHAPE "__shape__"
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384226040
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
Review comment:
is this also missing auxs ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r394499339
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,121 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = e.node->attrs.dict["__shape__"];
+ // add this shape to the list
+ ss << getShapeAt(shape, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
+ }
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = e.node->attrs.dict["__dtype__"];
+ // add this dtype to the list
+ ss << getDtypeAt(dtype, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
Review comment:
nit: use MX_STR_DTYPE instead of "__dtype__"
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247189
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
Review comment:
no, this is just an example implementation of the subgraph library. it can do whatever the user wants it to do.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-598691318
@ptrendx @Caenorst Can you guys please review this PR for TensorRT usage?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on issue #17623: Dynamic subgraph
compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-589834860
It sounds like the goal for passing data is to allow data to be compiled into the bin (example tensort bin) for that subgraph to avoid an init step.
If the weights are in the bin, then we structuring this such that weights can not be changed with calling optimize_for again using new weights ?
Is there any reason why we only do this for args and not auxs ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r381470169
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
added an example to the PR description
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-596044902
@leezu i think this PR is ready to go. But lets leave it open until next week, I was hoping to get a pass from the Nvidia folks for applicability for TensorRT usage. @ptrendx and @Caenorst
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r389204966
##########
File path: include/mxnet/lib_api.h
##########
@@ -234,10 +234,15 @@ enum MXReturnValue {
*/
struct MXTensor {
MXTensor() : data_ptr(nullptr), dtype(kUNSET), verID(0) {}
-
+ MXTensor(const MXTensor& oth) : data_ptr(oth.data_ptr), shape(oth.shape),
+ dtype(oth.dtype), verID(oth.verID), ctx(oth.ctx) {
+ setDLTensor();
Review comment:
then if it's copying we don't need the setTensor call below. should we just make setTensor call here?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247773
##########
File path: example/extensions/lib_subgraph/test_subgraph.py
##########
@@ -88,3 +91,54 @@
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
print(out4)
+# Gluon Hybridize partitioning with shapes/types without inference
+print('-------------------------------')
+print('Testing Gluon Hybridize partitioning with shapes/types without inference')
+inputs = [a,b]
+sym_block2 = nn.SymbolBlock(sym, inputs)
+sym_block2.initialize()
+sym_block2.optimize_for(mx.nd.ones((3,2)), mx.nd.ones((3,2)), backend='myProp')
+sym_block2.export('partitioned')
+
+
+###############################################
+# Test with subgraph directly consuming params
+###############################################
+# example model, ops to be partitioned have args
+d2 = mx.sym.exp(a)
Review comment:
this is just giving us a baseline to compare again built-in MXNet computation flow. and again its just an example.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384248544
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
auxName is not necessary.` argName` will be set for all "variables" which includes both args and aux.
for an arg param, isArg = True && argName = 'bob'
for an aux param, isArg = True && isAux = true && argName = 'bob'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384301289
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -205,7 +205,7 @@ MXReturnValue mySupportedOps(std::string json,
}
//check if op dtype is float
- if(dtype == kFloat32) {
+ if((dtype == kFloat32 && options.count("reqFloat") > 0) || options.count("reqFloat") == 0) {
Review comment:
changed the comment to clarify:
```
//check if op dtype is float, and if option was specified to require float types
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384248544
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
no. argName will be set for all "variables" which includes both args and aux. for an aux param, isArg = True && isAux = true && argName = 'bob'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247611
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
+ std::cout << "arg: " << kv.first << " ==> (";
+ for (auto s : kv.second.shape)
+ std::cout << s << ",";
+ std::cout << ") [";
+ for (int i=0; i<kv.second.size(); i++)
+ std::cout << kv.second.data<float>()[i] << ", ";
+ std::cout << "]" << std::endl;
+ }
+ if(options.count("reqArgs") > 0 && args.size() == 0) {
Review comment:
this is just an example implementation of the subgraph library. it can do whatever the user wants it to do. the `options` map contains whatever the user passes to the `optimize_for` APIs. Its purely a pass through from the user to the backend
Theres no reason to require aux by default, depending on the model it may or may not have any aux params. The smarter thing a user could do is check and see if the subgraph has aux inputs, and then in the aux map is empty error out. But again this is just an example subgraph library. Not the one that everyone must use as-is, unmodified.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247189
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
Review comment:
no, this is just an example implementation of the subgraph library. it can do whatever the user wants it to do. In fact, here we're just looping and printing out the args necessarily, so it doesnt do anything important. If a user wanted to print out aux too they could do so.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu merged pull request #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
leezu merged pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388629152
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
Review comment:
what if user calls hybridize and then call optimize_for? nothing will happen or it throws on unrelated error message?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384239630
##########
File path: perl-package/AI-MXNetCAPI/mxnet.i
##########
@@ -1633,6 +1633,8 @@ int MXOptimizeForBackend(SymbolHandle sym_handle,
const mx_uint in,
NDArrayHandle* in,
const mx_uint in,
+ NDArrayHandle* in,
Review comment:
should we have the same names for this function params?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247937
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
Review comment:
nope. in Gluon `args` here means inputs to the model. not the same as args/aux in symbol/module flow. These are the same function arguments as used in the `forward` API:
https://github.com/apache/incubator-mxnet/blob/13f5ad967992d92016667e8c9387ec128d43f01f/python/mxnet/gluon/block.py#L1155-L1158
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r381469932
##########
File path: src/c_api/c_api.cc
##########
@@ -823,9 +823,9 @@ int MXLoadLib(const char *path) {
// MXNET_REGISTER_SUBGRAPH_PROPERTY(customBackend, CustomSubgraphProperty);
mxnet::op::SubgraphBackendRegistry::Get()->__REGISTER_CUSTOM_PROPERTY__(name_str,
- std::make_shared<mxnet::op::CustomSubgraphProperty>(
- strategy_str, callSupportedOps, supportedOps_fp,
- callAcceptSubgraph, acceptSubgraph_fp, callFree, op_name_str));
+ std::make_shared<mxnet::op::CustomSubgraphProperty>(
Review comment:
sure, will do
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-588608779
What do you think @HahTK?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247937
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
Review comment:
nope. in Gluon args here means inputs to the model. not the same as args/aux in symbol/module flow
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388636626
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,69 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Partitions the current HybridBlock and optimizes it for a given backend
+ without executing a forward pass. Modifies the HybridBlock in-place.
+
+ Immediately partitions a HybridBlock using the specified backend. Combines
+ the work done in the hybridize API with part of the work done in the forward
+ pass without calling the CachedOp. Can be used in place of hybridize,
+ afterwards `export` can be called or inference can be run. See README.md in
+ example/extensions/lib_subgraph/README.md for more details.
+
+ Examples
+ --------
+ # partition and then export to file
+ block.optimize_for(x, backend='myPart')
+ block.export('partitioned')
+
+ # partition and then run inference
+ block.optimize_for(x, backend='myPart')
+ block(x)
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
+
+ # do part of forward API call
+ has_symbol, has_ndarray, ctx_set, _ = _gather_type_ctx_info([x] + list(args))
+ if has_symbol:
+ raise ValueError('Inputs must be NDArrays for the optimize_for API'
+ ' Please check the type of the args.\n')
+ if not has_symbol and not has_ndarray:
+ raise ValueError('In HybridBlock, there must be one NDArray as input.'
+ ' Please check the type of the args.\n')
+ if len(ctx_set) > 1:
+ raise ValueError('Find multiple contexts in the input, '
+ 'After hybridized, the HybridBlock only supports one input '
+ 'context. You can print the ele.ctx in the '
+ 'input arguments to inspect their contexts. '
+ 'Find all contexts = {}'.format(ctx_set))
+
+ self._build_cache(x, *args)
Review comment:
`_build_cache` combines (x, args) back into a single tuple:
https://github.com/apache/incubator-mxnet/blob/ade7d48fc7b977e28213ec24b628c61dc1c0c6f0/python/mxnet/gluon/block.py#L923
`_call_cached_op` does the same:
https://github.com/apache/incubator-mxnet/blob/ade7d48fc7b977e28213ec24b628c61dc1c0c6f0/python/mxnet/gluon/block.py#L993
So, passing (x, *args) to `_build_cache` results in just a single tuple args that contains everything inside the function.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388616931
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
Review comment:
just pushed the change, lmk if that addresses your concern
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247773
##########
File path: example/extensions/lib_subgraph/test_subgraph.py
##########
@@ -88,3 +91,54 @@
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
print(out4)
+# Gluon Hybridize partitioning with shapes/types without inference
+print('-------------------------------')
+print('Testing Gluon Hybridize partitioning with shapes/types without inference')
+inputs = [a,b]
+sym_block2 = nn.SymbolBlock(sym, inputs)
+sym_block2.initialize()
+sym_block2.optimize_for(mx.nd.ones((3,2)), mx.nd.ones((3,2)), backend='myProp')
+sym_block2.export('partitioned')
+
+
+###############################################
+# Test with subgraph directly consuming params
+###############################################
+# example model, ops to be partitioned have args
+d2 = mx.sym.exp(a)
Review comment:
Remember this is not a test, this an example python script showing different ways of partitioning the model. It prints out the results of forward just to show that the results are the same. The comparison to the regular symbol flow is just giving us a baseline to compare again built-in MXNet computation flow.
The actual tests are in tests/python/unittest/test_extensions.py
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384223762
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
Review comment:
are we missing auxs here ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388630243
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,69 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Partitions the current HybridBlock and optimizes it for a given backend
+ without executing a forward pass. Modifies the HybridBlock in-place.
+
+ Immediately partitions a HybridBlock using the specified backend. Combines
+ the work done in the hybridize API with part of the work done in the forward
+ pass without calling the CachedOp. Can be used in place of hybridize,
+ afterwards `export` can be called or inference can be run. See README.md in
+ example/extensions/lib_subgraph/README.md for more details.
+
+ Examples
+ --------
+ # partition and then export to file
+ block.optimize_for(x, backend='myPart')
+ block.export('partitioned')
+
+ # partition and then run inference
+ block.optimize_for(x, backend='myPart')
+ block(x)
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
+
+ # do part of forward API call
+ has_symbol, has_ndarray, ctx_set, _ = _gather_type_ctx_info([x] + list(args))
+ if has_symbol:
+ raise ValueError('Inputs must be NDArrays for the optimize_for API'
+ ' Please check the type of the args.\n')
+ if not has_symbol and not has_ndarray:
+ raise ValueError('In HybridBlock, there must be one NDArray as input.'
+ ' Please check the type of the args.\n')
+ if len(ctx_set) > 1:
+ raise ValueError('Find multiple contexts in the input, '
+ 'After hybridized, the HybridBlock only supports one input '
+ 'context. You can print the ele.ctx in the '
+ 'input arguments to inspect their contexts. '
+ 'Find all contexts = {}'.format(ctx_set))
+
+ self._build_cache(x, *args)
Review comment:
I know it is gluon norm to compress everything into this "*args", but I will appreciate if you can write a short comment on that since this "*args" won't have the same thing as _call_cached_op
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388627509
##########
File path: include/mxnet/lib_api.h
##########
@@ -234,10 +234,15 @@ enum MXReturnValue {
*/
struct MXTensor {
MXTensor() : data_ptr(nullptr), dtype(kUNSET), verID(0) {}
-
+ MXTensor(const MXTensor& oth) : data_ptr(oth.data_ptr), shape(oth.shape),
+ dtype(oth.dtype), verID(oth.verID), ctx(oth.ctx) {
+ setDLTensor();
Review comment:
why do we need setDLTensor in the constructor?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384692936
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
Review comment:
Agreed. I did not notice that this was an example
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393339392
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -308,6 +350,63 @@ class CustomSubgraphProperty: public SubgraphProperty {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ std::cout << "in InitSubgraphInputs" << std::endl;
Review comment:
remove this?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384302249
##########
File path: example/extensions/lib_subgraph/README.md
##########
@@ -120,11 +122,8 @@ There are several essential building blocks for making a custom partitioner:
* This macro registers the custom partitioner and its properties to MXNet by its name. Notice that a partitioner can have multiple partitioning strategies. This enables multiple *passes* to be run in a single partitioning call from the user. The first argument to `addStrategy` is a user-specified name. The second argument is the `supportedOps` function. The third argument is the name of the subgraph operator to create for each subgraph created during partitioning (see below for more info about subgraph operators). The `setReviewSubgraph` API registers a callback function that is called for each subgraph created during partitioning (more on this below). Notice that the first argument to this function is the strategy to associate with and the second argument is the `reviewSubgraph` function.
REGISTER_PARTITIONER(my_part_name)
- .addStrategy("strategy1",
- supportedOps,
- "_custom_subgraph_op")
- .setReviewSubgraph("strategy1",
- reviewSubgraph);
+ .addStrategy("strategy1", supportedOps, "_custom_subgraph_op")
+ .setReviewSubgraph("strategy1", reviewSubgraph);
Review comment:
thanks for the reminder, done
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247773
##########
File path: example/extensions/lib_subgraph/test_subgraph.py
##########
@@ -88,3 +91,54 @@
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
print(out4)
+# Gluon Hybridize partitioning with shapes/types without inference
+print('-------------------------------')
+print('Testing Gluon Hybridize partitioning with shapes/types without inference')
+inputs = [a,b]
+sym_block2 = nn.SymbolBlock(sym, inputs)
+sym_block2.initialize()
+sym_block2.optimize_for(mx.nd.ones((3,2)), mx.nd.ones((3,2)), backend='myProp')
+sym_block2.export('partitioned')
+
+
+###############################################
+# Test with subgraph directly consuming params
+###############################################
+# example model, ops to be partitioned have args
+d2 = mx.sym.exp(a)
Review comment:
Remember this is not a test, this an example python script showing different ways of partitioning the model. It prints out the results of forward just to show that the results are the same. The comparison to the regular symbol flow is just giving us a baseline to compare again built-in MXNet computation flow.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388636626
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,69 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Partitions the current HybridBlock and optimizes it for a given backend
+ without executing a forward pass. Modifies the HybridBlock in-place.
+
+ Immediately partitions a HybridBlock using the specified backend. Combines
+ the work done in the hybridize API with part of the work done in the forward
+ pass without calling the CachedOp. Can be used in place of hybridize,
+ afterwards `export` can be called or inference can be run. See README.md in
+ example/extensions/lib_subgraph/README.md for more details.
+
+ Examples
+ --------
+ # partition and then export to file
+ block.optimize_for(x, backend='myPart')
+ block.export('partitioned')
+
+ # partition and then run inference
+ block.optimize_for(x, backend='myPart')
+ block(x)
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
+
+ # do part of forward API call
+ has_symbol, has_ndarray, ctx_set, _ = _gather_type_ctx_info([x] + list(args))
+ if has_symbol:
+ raise ValueError('Inputs must be NDArrays for the optimize_for API'
+ ' Please check the type of the args.\n')
+ if not has_symbol and not has_ndarray:
+ raise ValueError('In HybridBlock, there must be one NDArray as input.'
+ ' Please check the type of the args.\n')
+ if len(ctx_set) > 1:
+ raise ValueError('Find multiple contexts in the input, '
+ 'After hybridized, the HybridBlock only supports one input '
+ 'context. You can print the ele.ctx in the '
+ 'input arguments to inspect their contexts. '
+ 'Find all contexts = {}'.format(ctx_set))
+
+ self._build_cache(x, *args)
Review comment:
`_build_cache` combines (x, args) back into a single tuple:
https://github.com/apache/incubator-mxnet/blob/ade7d48fc7b977e28213ec24b628c61dc1c0c6f0/python/mxnet/gluon/block.py#L923
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on issue #17623: Dynamic subgraph
compile support
Posted by GitBox <gi...@apache.org>.
leezu commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-600308561
@ptrendx any concerns on TensorRT or is this good to merge?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384791928
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
Heres an example from resnet's batchNorm operator:
```
{
"op": "null",
"name": "resnetv10_batchnorm0_beta0",
"attrs": {
"argName": "resnetv10_batchnorm0_beta",
"isArg": "True",
"isAux": "False"
},
"inputs": []
},
{
"op": "null",
"name": "resnetv10_batchnorm0_running_mean0",
"attrs": {
"argName": "resnetv10_batchnorm0_running_mean",
"isArg": "True",
"isAux": "True"
},
"inputs": []
},
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384242851
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -177,7 +177,7 @@ REGISTER_OP(_custom_subgraph_op)
const std::vector<std::string> op_names({"exp","log"});
MXReturnValue mySupportedOps(std::string json,
- std::vector<bool> ids,
+ std::vector<bool>& ids,
Review comment:
can you elaborate this ids in either the code or the doc? like "this vector flags all the ops in the given symbol graph to be partitioned into subgraphs"
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393451877
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
Review comment:
modify logic for the case when `n` is a subgraph node and `shape` a list of lists
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-587994956
If #17585 gets merged first, then ill update the doc with the new compile info. If this one gets merged first ill got modify #17585 to add the description.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r394499339
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,121 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = e.node->attrs.dict["__shape__"];
+ // add this shape to the list
+ ss << getShapeAt(shape, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
+ }
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = e.node->attrs.dict["__dtype__"];
+ // add this dtype to the list
+ ss << getDtypeAt(dtype, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
Review comment:
nit: use MX_STR_DTYPE instead of `"__dtype__"`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-596038638
@mxnet-label-bot update [pr-awaiting-merge]
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393345958
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -308,6 +350,63 @@ class CustomSubgraphProperty: public SubgraphProperty {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ std::cout << "in InitSubgraphInputs" << std::endl;
Review comment:
durrr....thanks. just pushed the change
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r392558369
##########
File path: src/c_api/c_api.cc
##########
@@ -122,14 +122,28 @@ void CustomFComputeDispatcher(const std::string op_name,
// convert inputs/outpus NDArray to C types to be passed to lib_api.h
for (size_t i = 0; i < inputs.size(); i++) {
- in_data.push_back(inputs[i].data().dptr_);
- in_shapes.push_back(inputs[i].shape().data());
- in_dims.push_back(inputs[i].shape().ndim());
- in_types.push_back(inputs[i].dtype());
- in_verIDs.push_back(inputs[i].version());
- const char* ctx_str = inputs[i].ctx().dev_mask() == Context::kCPU ? "cpu" : "gpu";
- in_dev_type.push_back(ctx_str);
- in_dev_id.push_back(inputs[i].ctx().real_dev_id());
+ const NDArray& in_nd = inputs[i];
+ // reorder data if in MKLDNN format
+ if (in_nd.IsMKLDNNData()) {
+ const NDArray& tmp_nd = in_nd.Reorder2Default();
Review comment:
i pushed a change to use pointers instead so that the code isnt duplicated, let me know how that looks to you
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388631246
##########
File path: src/c_api/c_api_symbolic.cc
##########
@@ -1383,7 +1403,26 @@ int MXOptimizeForBackend(SymbolHandle sym_handle,
common::HandleInferStorageTypeError(num_forward_inputs, indexed_graph,
g.GetAttr<StorageTypeVector>("storage_type"));
}
+ std::vector<std::string> arg_names = sym->ListInputNames(nnvm::Symbol::kReadOnlyArgs);
+ g.attrs["in_args"] = std::make_shared<nnvm::any>(in_args_ptr);
+ g.attrs["in_arg_names"] = std::make_shared<nnvm::any>(arg_names);
+
+ std::vector<std::string> aux_names = sym->ListInputNames(nnvm::Symbol::kAuxiliaryStates);
+ g.attrs["in_aux"] = std::make_shared<nnvm::any>(in_aux_ptr);
+ g.attrs["in_aux_names"] = std::make_shared<nnvm::any>(aux_names);
+ } else {
+ NDArray **in_args_ptr = static_cast<NDArray**>(nullptr);
+ std::vector<std::string> arg_names;
+ g.attrs["in_args"] = std::make_shared<nnvm::any>(in_args_ptr);
+ g.attrs["in_arg_names"] = std::make_shared<nnvm::any>(arg_names);
+
+ NDArray **in_aux_ptr = static_cast<NDArray**>(nullptr);
+ std::vector<std::string> aux_names;
+ g.attrs["in_aux"] = std::make_shared<nnvm::any>(in_aux_ptr);
+ g.attrs["in_aux_names"] = std::make_shared<nnvm::any>(aux_names);
}
+
+
Review comment:
remove redundant line 🌵
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-591282169
Thanks for the feedback @HahTK and @rondogency! I made some changes and replied to your comments/suggestions
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-600787356
@leezu, Reviewed offline with @ptrendx and @Caenorst. Decided that there are a few missing features needed for TensorRT to use this compile API. But decided to list them here https://github.com/apache/incubator-mxnet/issues/17236 and implement them in a subsequent PR. We can go ahead and merge this one as it is still useful for compiling for TVM.
After this PR is merged i'll open another PR and start working on the missing features next.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388556154
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
Review comment:
I think that's reasonable as a separate paragraph in the docstring. But it'd be too confusing as a sentence summary. Maybe for the first sentence, you could describe what optimize_for is used for. Ie. enable users to optimize for a backend without doing an actual forward pass. The concept of CachedOp may not be clear to users.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247937
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
Review comment:
It is not missing aux. in Gluon `args` here means inputs to the model. not the same as args/aux in symbol/module flow. These are the same function arguments as used in the `forward` API:
https://github.com/apache/incubator-mxnet/blob/13f5ad967992d92016667e8c9387ec128d43f01f/python/mxnet/gluon/block.py#L1155-L1158
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-589858094
> It sounds like the goal for passing data is to allow data to be compiled into the bin (example tensort bin) for that subgraph to avoid an init step.
>
> If the weights are in the bin, then we structuring this such that weights can not be changed with calling optimize_for again using new weights ?
Using the weights in `optimize_for` is a one-way flow. You cannot call `optimize_for` again with new weights. You would need to call it on the original graph with new weights. The assumption is that weights would only be used for inference. So presumably the model is already trained, and the weights are frozen.
> Is there any reason why we only do this for args and not auxs ?
Thanks for pointing this out. I'll add them too
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r381451652
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
where is "argName" getting used?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388636221
##########
File path: include/mxnet/lib_api.h
##########
@@ -234,10 +234,15 @@ enum MXReturnValue {
*/
struct MXTensor {
MXTensor() : data_ptr(nullptr), dtype(kUNSET), verID(0) {}
-
+ MXTensor(const MXTensor& oth) : data_ptr(oth.data_ptr), shape(oth.shape),
+ dtype(oth.dtype), verID(oth.verID), ctx(oth.ctx) {
+ setDLTensor();
Review comment:
In this case, its the copy constructor. So we're constructing an MXTensor from another one, and we need to setup the DLTensor for the new one that we're constructing. This is used in std::map
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388508840
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
Review comment:
The meaning of `optimize_for` API in Symbol class is to partition the model. In Gluon to partition we have to hybridize to build the symbol, but then hybridize doesnt actually do anything. It just sets up for doing the actual symbol creation, which is actually executed as part of the `forward` call. And then we still need to partition it. So it made sense to integrate that inside as a single step for the user.
Especially since partitioning requires `active` to set to `True` so it didnt make sense to let users call hybridize with `active` as `False` since we couldnt build the symbol. Just removed a possible confusion point for users.
GluonCV does a similar thing with its `export_block` API where they call hybridize under the hood instead to provide a better user experience with a single API call.
https://github.com/dmlc/gluon-cv/blob/9921323263d3698ce3e03a9eb4ada208b7b8bce3/gluoncv/utils/export_helper.py#L129-L133
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393486989
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
+ ss << shape.substr(1, shape.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
}
- // free memory used by custom op to allocate attributes
- call_free_(attr_vals);
- call_free_(attr_keys);
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = n->attrs.dict["__dtype__"];
+ ss << dtype.substr(1, dtype.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
+ }
+ // set user specified attributes
+ for (auto attr : user_attrs)
+ n->attrs.dict[attr.first] = attr.second;
+
return n;
} else {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ for (size_t i = 0; i < input_entries->size(); ++i) {
+ nnvm::NodeEntry *e = input_entries->at(i);
+ nnvm::NodeEntry& orig = orig_input_entries->at(i);
+
+ // set attribute for subgraph input to indicate if it is from an arg/param to model
+ if (orig.node->is_variable()) {
+ // get name of original output entry
+ nnvm::Symbol sym;
+ sym.outputs.push_back(orig);
+ const auto output_names = sym.ListOutputNames();
+ CHECK_EQ(output_names.size(), 1U);
+ const std::string& var_name = output_names[0];
+
+ e->node->attrs.dict["isArg"] = "True";
+ e->node->attrs.dict["argName"] = var_name;
+ } else {
+ e->node->attrs.dict["isArg"] = "False";
+ }
+
+ // pass down other attributes if available
+ if (orig.node->attrs.dict.count("__dtype__") > 0) {
+ // get dtype string from other node
+ std::string& dtype = orig.node->attrs.dict["__dtype__"];
+ int idx = 0;
+ // find the beginning of the output dtype for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = dtype.find("[", idx+1);
+ if (idx == 0) idx++; // if output index is 0, start after first square bracket [
+ int stop = dtype.find("]", idx); // find stop index for this output dtype
+ std::stringstream ss;
+ // create new dtype string for this node
+ ss << "[" << dtype.substr(idx, stop-idx+1) << "]";
+ e->node->attrs.dict["__dtype__"] = ss.str();
+ }
+
+ if (orig.node->attrs.dict.count("__shape__") > 0) {
+ // get shape string from other node
+ std::string& shape = orig.node->attrs.dict["__shape__"];
+ int idx = 0;
+ // find the beginning of the output shape for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = shape.find("[", idx+1);
Review comment:
fixed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384302001
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -177,7 +177,7 @@ REGISTER_OP(_custom_subgraph_op)
const std::vector<std::string> op_names({"exp","log"});
MXReturnValue mySupportedOps(std::string json,
- std::vector<bool> ids,
+ std::vector<bool>& ids,
Review comment:
In the doc it says
> The 2nd argument is an array of booleans, one for each operator in the model. When traversing the graph, operators to be partitioned into subgraphs are identified and an entry is set to `true` for the index in the `ids` array corresponding to the node ID.
Is that sufficient?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247937
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
Review comment:
nope. in Gluon `args` here means inputs to the model. not the same as args/aux in symbol/module flow
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388643662
##########
File path: src/c_api/c_api_symbolic.cc
##########
@@ -1383,7 +1403,26 @@ int MXOptimizeForBackend(SymbolHandle sym_handle,
common::HandleInferStorageTypeError(num_forward_inputs, indexed_graph,
g.GetAttr<StorageTypeVector>("storage_type"));
}
+ std::vector<std::string> arg_names = sym->ListInputNames(nnvm::Symbol::kReadOnlyArgs);
+ g.attrs["in_args"] = std::make_shared<nnvm::any>(in_args_ptr);
+ g.attrs["in_arg_names"] = std::make_shared<nnvm::any>(arg_names);
+
+ std::vector<std::string> aux_names = sym->ListInputNames(nnvm::Symbol::kAuxiliaryStates);
+ g.attrs["in_aux"] = std::make_shared<nnvm::any>(in_aux_ptr);
+ g.attrs["in_aux_names"] = std::make_shared<nnvm::any>(aux_names);
+ } else {
+ NDArray **in_args_ptr = static_cast<NDArray**>(nullptr);
+ std::vector<std::string> arg_names;
+ g.attrs["in_args"] = std::make_shared<nnvm::any>(in_args_ptr);
+ g.attrs["in_arg_names"] = std::make_shared<nnvm::any>(arg_names);
+
+ NDArray **in_aux_ptr = static_cast<NDArray**>(nullptr);
+ std::vector<std::string> aux_names;
+ g.attrs["in_aux"] = std::make_shared<nnvm::any>(in_aux_ptr);
+ g.attrs["in_aux_names"] = std::make_shared<nnvm::any>(aux_names);
}
+
+
Review comment:
it needed two lines because the code above and below were fighting and couldnt stand eachother, so it needs some extra room ;-)
Removed it in the latest commit
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384239088
##########
File path: example/extensions/lib_subgraph/README.md
##########
@@ -120,11 +122,8 @@ There are several essential building blocks for making a custom partitioner:
* This macro registers the custom partitioner and its properties to MXNet by its name. Notice that a partitioner can have multiple partitioning strategies. This enables multiple *passes* to be run in a single partitioning call from the user. The first argument to `addStrategy` is a user-specified name. The second argument is the `supportedOps` function. The third argument is the name of the subgraph operator to create for each subgraph created during partitioning (see below for more info about subgraph operators). The `setReviewSubgraph` API registers a callback function that is called for each subgraph created during partitioning (more on this below). Notice that the first argument to this function is the strategy to associate with and the second argument is the `reviewSubgraph` function.
REGISTER_PARTITIONER(my_part_name)
- .addStrategy("strategy1",
- supportedOps,
- "_custom_subgraph_op")
- .setReviewSubgraph("strategy1",
- reviewSubgraph);
+ .addStrategy("strategy1", supportedOps, "_custom_subgraph_op")
+ .setReviewSubgraph("strategy1", reviewSubgraph);
Review comment:
don't forget the add gluon optimize_for you just wrote to this doc
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-590388983
@HahTK ive added the ability to pass params down for aux args, and your suggestion to enable removing the params to the enhancements issue: #17236
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393449318
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
+ ss << shape.substr(1, shape.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
}
- // free memory used by custom op to allocate attributes
- call_free_(attr_vals);
- call_free_(attr_keys);
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = n->attrs.dict["__dtype__"];
+ ss << dtype.substr(1, dtype.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
+ }
+ // set user specified attributes
+ for (auto attr : user_attrs)
+ n->attrs.dict[attr.first] = attr.second;
+
return n;
} else {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ for (size_t i = 0; i < input_entries->size(); ++i) {
+ nnvm::NodeEntry *e = input_entries->at(i);
+ nnvm::NodeEntry& orig = orig_input_entries->at(i);
+
+ // set attribute for subgraph input to indicate if it is from an arg/param to model
+ if (orig.node->is_variable()) {
+ // get name of original output entry
+ nnvm::Symbol sym;
+ sym.outputs.push_back(orig);
+ const auto output_names = sym.ListOutputNames();
+ CHECK_EQ(output_names.size(), 1U);
+ const std::string& var_name = output_names[0];
+
+ e->node->attrs.dict["isArg"] = "True";
+ e->node->attrs.dict["argName"] = var_name;
+ } else {
+ e->node->attrs.dict["isArg"] = "False";
+ }
+
+ // pass down other attributes if available
+ if (orig.node->attrs.dict.count("__dtype__") > 0) {
+ // get dtype string from other node
+ std::string& dtype = orig.node->attrs.dict["__dtype__"];
+ int idx = 0;
+ // find the beginning of the output dtype for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = dtype.find("[", idx+1);
+ if (idx == 0) idx++; // if output index is 0, start after first square bracket [
+ int stop = dtype.find("]", idx); // find stop index for this output dtype
+ std::stringstream ss;
+ // create new dtype string for this node
+ ss << "[" << dtype.substr(idx, stop-idx+1) << "]";
+ e->node->attrs.dict["__dtype__"] = ss.str();
+ }
+
+ if (orig.node->attrs.dict.count("__shape__") > 0) {
+ // get shape string from other node
+ std::string& shape = orig.node->attrs.dict["__shape__"];
+ int idx = 0;
+ // find the beginning of the output shape for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = shape.find("[", idx+1);
Review comment:
check if `idx` points to the right index
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r381449945
##########
File path: src/c_api/c_api.cc
##########
@@ -823,9 +823,9 @@ int MXLoadLib(const char *path) {
// MXNET_REGISTER_SUBGRAPH_PROPERTY(customBackend, CustomSubgraphProperty);
mxnet::op::SubgraphBackendRegistry::Get()->__REGISTER_CUSTOM_PROPERTY__(name_str,
- std::make_shared<mxnet::op::CustomSubgraphProperty>(
- strategy_str, callSupportedOps, supportedOps_fp,
- callAcceptSubgraph, acceptSubgraph_fp, callFree, op_name_str));
+ std::make_shared<mxnet::op::CustomSubgraphProperty>(
Review comment:
looks like some unintentional spacing changes were made. can you please fix them if you end up pushing another commit?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384223932
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
+ std::cout << "arg: " << kv.first << " ==> (";
+ for (auto s : kv.second.shape)
+ std::cout << s << ",";
+ std::cout << ") [";
+ for (int i=0; i<kv.second.size(); i++)
+ std::cout << kv.second.data<float>()[i] << ", ";
+ std::cout << "]" << std::endl;
+ }
+ if(options.count("reqArgs") > 0 && args.size() == 0) {
Review comment:
ditto missing auxs again ?
we should probably have a reqAuxs option too
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-590577917
@sergeykolychev I had to update the optimize_for API call in the perl bindings. Can you please review/approve for the change here:
https://github.com/apache/incubator-mxnet/pull/17623/files#diff-cc128e9c950abe76c19162ce01fe2402
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r394499196
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,121 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = e.node->attrs.dict["__shape__"];
+ // add this shape to the list
+ ss << getShapeAt(shape, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
Review comment:
nit: use MX_STR_SHAPE instead of "__shape__"
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247189
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
Review comment:
were not missing aux here, this is just an example implementation of the subgraph library. it can do whatever the user wants it to do. In fact, here we're just looping and printing out the args unnecessarily, so it doesnt do anything important. If a user wanted to print out aux too they could do so.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384792756
##########
File path: src/operator/subgraph/build_subgraph.cc
##########
@@ -561,10 +561,12 @@ void CutGraphInputs(const std::vector<nnvm::NodeEntry*> &input_entries,
nnvm::ObjectPtr n = nnvm::CreateVariableNode(
var_name + std::to_string(name_count_map[var_name]));
// set attribute for subgraph input to indicate if it is from an arg/param to model
- if (e->node->is_variable())
+ if (e->node->is_variable()) {
n->attrs.dict["isArg"] = "True";
- else
+ n->attrs.dict["argName"] = var_name;
Review comment:
Heres an example of an input to a subgraph thats an output from another operator (ie. not a param):
```
{
"op": "null",
"name": "_plus0_output0",
"attrs": {
"isArg": "False",
"isAux": "False"
},
"inputs": []
},
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388636626
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,69 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Partitions the current HybridBlock and optimizes it for a given backend
+ without executing a forward pass. Modifies the HybridBlock in-place.
+
+ Immediately partitions a HybridBlock using the specified backend. Combines
+ the work done in the hybridize API with part of the work done in the forward
+ pass without calling the CachedOp. Can be used in place of hybridize,
+ afterwards `export` can be called or inference can be run. See README.md in
+ example/extensions/lib_subgraph/README.md for more details.
+
+ Examples
+ --------
+ # partition and then export to file
+ block.optimize_for(x, backend='myPart')
+ block.export('partitioned')
+
+ # partition and then run inference
+ block.optimize_for(x, backend='myPart')
+ block(x)
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
+
+ # do part of forward API call
+ has_symbol, has_ndarray, ctx_set, _ = _gather_type_ctx_info([x] + list(args))
+ if has_symbol:
+ raise ValueError('Inputs must be NDArrays for the optimize_for API'
+ ' Please check the type of the args.\n')
+ if not has_symbol and not has_ndarray:
+ raise ValueError('In HybridBlock, there must be one NDArray as input.'
+ ' Please check the type of the args.\n')
+ if len(ctx_set) > 1:
+ raise ValueError('Find multiple contexts in the input, '
+ 'After hybridized, the HybridBlock only supports one input '
+ 'context. You can print the ele.ctx in the '
+ 'input arguments to inspect their contexts. '
+ 'Find all contexts = {}'.format(ctx_set))
+
+ self._build_cache(x, *args)
Review comment:
`_build_cache` combines (x, args) back into a single tuple:
https://github.com/apache/incubator-mxnet/blob/ade7d48fc7b977e28213ec24b628c61dc1c0c6f0/python/mxnet/gluon/block.py#L923
`_call_cached_op` does the same:
https://github.com/apache/incubator-mxnet/blob/ade7d48fc7b977e28213ec24b628c61dc1c0c6f0/python/mxnet/gluon/block.py#L993
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384693040
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
+ std::cout << "arg: " << kv.first << " ==> (";
+ for (auto s : kv.second.shape)
+ std::cout << s << ",";
+ std::cout << ") [";
+ for (int i=0; i<kv.second.size(); i++)
+ std::cout << kv.second.data<float>()[i] << ", ";
+ std::cout << "]" << std::endl;
+ }
+ if(options.count("reqArgs") > 0 && args.size() == 0) {
Review comment:
Agreed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384301444
##########
File path: perl-package/AI-MXNetCAPI/mxnet.i
##########
@@ -1633,6 +1633,8 @@ int MXOptimizeForBackend(SymbolHandle sym_handle,
const mx_uint in,
NDArrayHandle* in,
const mx_uint in,
+ NDArrayHandle* in,
Review comment:
I was just following the style that was already there. @sergeykolychev to verify
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r394499196
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,121 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ const nnvm::NodeEntry& e = sym.outputs[i];
+ if (e.node->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = e.node->attrs.dict["__shape__"];
+ // add this shape to the list
+ ss << getShapeAt(shape, e.index);
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
Review comment:
nit: use MX_STR_SHAPE instead of `"__shape__"`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388460612
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
Review comment:
Should this docstring be extended to clarify the difference with `hybridize`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] leezu commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
leezu commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388461145
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
Review comment:
Could you elaborate on your choice of `optimize_for` calling `hybridize`, instead of requiring users to call `hybridize` and then `optimize_for`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388632121
##########
File path: tests/python/unittest/test_extensions.py
##########
@@ -167,3 +167,17 @@ def test_subgraph():
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
# check that result matches one executed by MXNet
assert_almost_equal(out[0].asnumpy(), out4[0].asnumpy(), rtol=1e-3, atol=1e-3)
+
+ # Gluon Hybridize partitioning with shapes/types
+ sym_block2 = nn.SymbolBlock(sym, [a,b])
+ sym_block2.initialize()
+ a_data = mx.nd.ones((3,2))
+ b_data = mx.nd.ones((3,2))
Review comment:
I just realized you didn't do the randomized inputs like the custom op test, I am thinking if it might be risky
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384239275
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -205,7 +205,7 @@ MXReturnValue mySupportedOps(std::string json,
}
//check if op dtype is float
- if(dtype == kFloat32) {
+ if((dtype == kFloat32 && options.count("reqFloat") > 0) || options.count("reqFloat") == 0) {
Review comment:
what is "reqFloat"?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r384247611
##########
File path: example/extensions/lib_subgraph/subgraph_lib.cc
##########
@@ -218,10 +218,27 @@ MXReturnValue mySupportedOps(std::string json,
MXReturnValue myReviewSubgraph(std::string json, int subraph_id, bool* accept,
std::unordered_map<std::string, std::string>& options,
- std::unordered_map<std::string, std::string>& attrs) {
+ std::unordered_map<std::string, std::string>& attrs,
+ std::map<std::string, MXTensor>& args,
+ std::map<std::string, MXTensor>& aux) {
for (auto kv : options) {
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
}
+ for (auto kv : args) {
+ std::cout << "arg: " << kv.first << " ==> (";
+ for (auto s : kv.second.shape)
+ std::cout << s << ",";
+ std::cout << ") [";
+ for (int i=0; i<kv.second.size(); i++)
+ std::cout << kv.second.data<float>()[i] << ", ";
+ std::cout << "]" << std::endl;
+ }
+ if(options.count("reqArgs") > 0 && args.size() == 0) {
Review comment:
this is just an example implementation of the subgraph library. it can do whatever the user wants it to do.
Theres no reason to require aux by default, depending on the model it may or may not have any aux params. The smarter thing a user could do is check and see if the subgraph has aux inputs, and then in the aux map is empty error out.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388637623
##########
File path: python/mxnet/symbol/symbol.py
##########
@@ -1475,13 +1482,19 @@ def optimize_for(self, backend, args=None, ctx=None, **kwargs):
out = SymbolHandle()
assert isinstance(backend, str)
- if args is None:
+ if args is None or len(args) == 0:
args = []
args_handle = c_array(NDArrayHandle, [])
else:
- listed_arguments = self.list_arguments()
- args_handle, args = self._get_ndarray_inputs('args', args, listed_arguments, False)
+ args_handle, args = self._get_ndarray_inputs('args', args,
+ self.list_arguments(), False)
+ if aux is None or len(aux) == 0:
+ aux = []
Review comment:
Yes, because later on line 1513 we do `mx_uint(len(aux))`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388508840
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
Review comment:
The meaning of `optimize_for` API in Symbol class is to partition the model. In Gluon to partition we have to hybridize to build the symbol, and then partition it. So it made sense to integrate that inside as a single step for the user.
GluonCV does a similar thing with its `export_block` API where they call hybridize under the hood instead to provide a better user experience with a single API call.
https://github.com/dmlc/gluon-cv/blob/9921323263d3698ce3e03a9eb4ada208b7b8bce3/gluoncv/utils/export_helper.py#L129-L133
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] rondogency commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
rondogency commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388633030
##########
File path: include/mxnet/lib_api.h
##########
@@ -407,6 +412,8 @@ class OpResource {
*/
/*! \brief Macro to help passing serialized subgraph through attribute dict */
#define SUBGRAPH_SYM_JSON "subgraph_sym_json"
+#define MX_DTYPE "__dtype__"
+#define MX_SHAPE "__shape__"
Review comment:
should we call it MX_STR_DTYPE? this name is too general and may cause some naming conflict
Also I seriously consider we should namespace all the primitives in lib_api.h, but it requires more work and we don't need to prioritize it now
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388643104
##########
File path: tests/python/unittest/test_extensions.py
##########
@@ -167,3 +167,17 @@ def test_subgraph():
out4 = sym_block(mx.nd.ones((3,2)),mx.nd.ones((3,2)))
# check that result matches one executed by MXNet
assert_almost_equal(out[0].asnumpy(), out4[0].asnumpy(), rtol=1e-3, atol=1e-3)
+
+ # Gluon Hybridize partitioning with shapes/types
+ sym_block2 = nn.SymbolBlock(sym, [a,b])
+ sym_block2.initialize()
+ a_data = mx.nd.ones((3,2))
+ b_data = mx.nd.ones((3,2))
Review comment:
I dont think it matters here since we're not testing any mathematical code. Just partitioning, we're not adding any operators.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on issue #17623: Dynamic
subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-600294991
> @mseth10 thanks! lmk what you think of the latest commit
All latest changes look good to me. Thanks @samskalicky for addressing multiple subgraph issues in this PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] HahTK commented on issue #17623: Dynamic subgraph
compile support
Posted by GitBox <gi...@apache.org>.
HahTK commented on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-590531101
@samskalicky :
I'll let the reviewers go through the code in detail but from perusal and with the aux added the feature looks good.
Things to think about in the future :
1. For backends with very long compile time, a weight_update function may be useful.
It also serves as a path to dynamically swap models (if so desired).
2. Some backends may absorb the weights in to the bin itself.
For those we should provide an option for the partitioner to remove the redundant weight nodes that have been fully absorbed into bins.
The corner case would of course be weights that are only partly absorbed.
This would reduce memory footprint and improve speed by avoid the need to shove around unused weights
I think both of these items can be done in the future but just enumerating them here
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky edited a comment on issue #17623:
Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky edited a comment on issue #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#issuecomment-600787356
@leezu, Reviewed offline with @ptrendx and @Caenorst. Decided that there are a few missing features needed for TensorRT to use this compile API. But decided to list them here https://github.com/apache/incubator-mxnet/issues/17236 and implement them in a subsequent PR (#17885). We can go ahead and merge this one as it is still useful for compiling for TVM.
After this PR is merged i'll open another PR and start working on the missing features next.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] samskalicky commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
samskalicky commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r388635890
##########
File path: python/mxnet/gluon/block.py
##########
@@ -1026,6 +1030,53 @@ def _call_cached_op(self, *args):
out = [out]
return _regroup(out, self._out_format)
+ def optimize_for(self, x, *args, backend=None, backend_opts=None, **kwargs):
+ """Activates or deactivates :py:class:`HybridBlock` s recursively. Has no effect on
+ non-hybrid children.
+
+ Parameters
+ ----------
+ x : NDArray
+ first input to model
+ *args : NDArray
+ other inputs to model
+ backend : str
+ The name of backend, as registered in `SubgraphBackendRegistry`, default None
+ backend_opts : dict of user-specified options to pass to the backend for partitioning, optional
+ Passed on to `PrePartition` and `PostPartition` functions of `SubgraphProperty`
+ static_alloc : bool, default False
+ Statically allocate memory to improve speed. Memory usage may increase.
+ static_shape : bool, default False
+ Optimize for invariant input shapes between iterations. Must also
+ set static_alloc to True. Change of input shapes is still allowed
+ but slower.
+ """
+
+ # do hybrize API call
+ self.hybridize(True, backend, backend_opts, **kwargs)
Review comment:
calling `hybridize` and then `optimize_for` is fine. Since `optimize_for` will just call `hybridize` again internally anyway.
Calling `optimize_for` and then `hybridize` though will reset the cachedOp since `hybridize` calls `_clear_cached_op`. So not a problem per-se. User just needs to realize what they are doing.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] mseth10 commented on a change in pull request
#17623: Dynamic subgraph compile support
Posted by GitBox <gi...@apache.org>.
mseth10 commented on a change in pull request #17623: Dynamic subgraph compile support
URL: https://github.com/apache/incubator-mxnet/pull/17623#discussion_r393450509
##########
File path: src/operator/subgraph/partitioner/custom_subgraph_property.h
##########
@@ -188,31 +278,133 @@ class CustomSubgraphProperty: public SubgraphProperty {
}
std::string subgraph_json = nnvm::pass::SaveJSON(g);
- CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(),
- subgraph_id, &accept, opt_keys_.data(),
- opt_vals_.data(), opt_keys_.size(),
- &attr_keys, &attr_vals, &num_attr))
+ CHECK(call_review_subgraph_(review_subgraph_, subgraph_json.c_str(), subgraph_id,
+ &accept, opt_keys_.data(), opt_vals_.data(),
+ opt_keys_.size(), &attr_keys, &attr_vals, &num_attr,
+ arg_names.data(), arg_names.size(), arg_data.data(),
+ arg_shapes.data(), arg_dims.data(), arg_types.data(),
+ arg_verIDs.data(), arg_dev_type.data(),
+ arg_dev_id.data(), aux_names.data(), aux_names.size(),
+ aux_data.data(), aux_shapes.data(), aux_dims.data(),
+ aux_types.data(), aux_verIDs.data(),
+ aux_dev_type.data(), aux_dev_id.data()))
<< "Error calling review_subgraph for '" << subgraph_prop << "'";
+
+ if (num_attr > 0) {
+ // set user specified attributes
+ for (int i=0; i < num_attr; i++) {
+ user_attrs[attr_keys[i]] = attr_vals[i];
+ call_free_(attr_vals[i]);
+ call_free_(attr_keys[i]);
+ }
+ // free memory used by custom op to allocate attributes
+ call_free_(attr_vals);
+ call_free_(attr_keys);
+ }
}
+
if (accept) {
nnvm::ObjectPtr n = nnvm::Node::Create();
n->attrs.op = Op::Get(subgraph_op_name);
n->attrs.name = "_op" + std::to_string(subgraph_id);
n->attrs.subgraphs.push_back(std::make_shared<nnvm::Symbol>(sym));
- // set user specified attributes
- for (int i=0; i < num_attr; i++) {
- n->attrs.dict[attr_keys[i]] = attr_vals[i];
- call_free_(attr_vals[i]);
- call_free_(attr_keys[i]);
+
+ // set shapes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__shape__") > 0) {
+ std::string& shape = n->attrs.dict["__shape__"];
+ ss << shape.substr(1, shape.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__shape__"] = ss.str();
}
- // free memory used by custom op to allocate attributes
- call_free_(attr_vals);
- call_free_(attr_keys);
+ // set dtypes
+ {
+ std::stringstream ss;
+ ss << "[";
+ for (unsigned i=0; i < sym.outputs.size(); i++) {
+ nnvm::Node* n = sym.outputs[i].node.get();
+ if (n->attrs.dict.count("__dtype__") > 0) {
+ std::string& dtype = n->attrs.dict["__dtype__"];
+ ss << dtype.substr(1, dtype.length()-2); // strip off outer square brackets []
+ }
+ if (i < sym.outputs.size()-1)
+ ss << ",";
+ }
+ ss << "]";
+ n->attrs.dict["__dtype__"] = ss.str();
+ }
+ // set user specified attributes
+ for (auto attr : user_attrs)
+ n->attrs.dict[attr.first] = attr.second;
+
return n;
} else {
return nullptr;
}
}
+
+ virtual void InitSubgraphInputs(std::vector<nnvm::NodeEntry*>* input_entries,
+ std::vector<nnvm::NodeEntry>* orig_input_entries) const {
+ for (size_t i = 0; i < input_entries->size(); ++i) {
+ nnvm::NodeEntry *e = input_entries->at(i);
+ nnvm::NodeEntry& orig = orig_input_entries->at(i);
+
+ // set attribute for subgraph input to indicate if it is from an arg/param to model
+ if (orig.node->is_variable()) {
+ // get name of original output entry
+ nnvm::Symbol sym;
+ sym.outputs.push_back(orig);
+ const auto output_names = sym.ListOutputNames();
+ CHECK_EQ(output_names.size(), 1U);
+ const std::string& var_name = output_names[0];
+
+ e->node->attrs.dict["isArg"] = "True";
+ e->node->attrs.dict["argName"] = var_name;
+ } else {
+ e->node->attrs.dict["isArg"] = "False";
+ }
+
+ // pass down other attributes if available
+ if (orig.node->attrs.dict.count("__dtype__") > 0) {
+ // get dtype string from other node
+ std::string& dtype = orig.node->attrs.dict["__dtype__"];
+ int idx = 0;
+ // find the beginning of the output dtype for the particular output index
+ for (unsigned x=0; x < orig.index; x++)
+ idx = dtype.find("[", idx+1);
Review comment:
modify this for list of integers
----------------------------------------------------------------
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
With regards,
Apache Git Services