gluon-nlp icon indicating copy to clipboard operation
gluon-nlp copied to clipboard

bertpass_gpu.cc does not support MXNet 1.8

Open leezu opened this issue 3 years ago • 3 comments

Due to API change https://github.com/apache/incubator-mxnet/issues/19135

leezu avatar Oct 08 '20 20:10 leezu

Thanks @leezu. The custom pass in https://github.com/dmlc/gluon-nlp/blob/v0.10.x/scripts/bert/bertpass_gpu.cc uses some custom code that @MoisesHer and I worked on post-1.7.0 release that eventually became the default in 1.8.0. Thanks to this collaboration, in 1.8 all lines 30-248 have been integration into lib_api.h so they can be removed from bertpass_gpu.cc.

Plus, we eliminated the extra step of taking the symbol_json string and converting it to a graph object in https://github.com/dmlc/gluon-nlp/blob/3fbe9619d9e68bc665f73c8cdf683213c6edd4d6/scripts/bert/bertpass_gpu.cc#L251-L262 So now the API will be simplified to: https://github.com/apache/incubator-mxnet/blob/cc4b8ec68b6ec9dae73046e9c34ac97439efda83/example/extensions/lib_pass/pass_lib.cc#L34-L35

MXReturnValue myPass(mxnet::ext::Graph *g,
                     const std::unordered_map<std::string, std::string>& options) {

So little things like changing how you loop over the nodes in the graph from: https://github.com/dmlc/gluon-nlp/blob/3fbe9619d9e68bc665f73c8cdf683213c6edd4d6/scripts/bert/bertpass_gpu.cc#L266 will need to be changed to: https://github.com/apache/incubator-mxnet/blob/cc4b8ec68b6ec9dae73046e9c34ac97439efda83/example/extensions/lib_subgraph/subgraph_lib.cc#L308-L309

  for(int i=0; i<graph->size(); i++) {
    mxnet::ext::Node* n = graph->getNode(i);

And creating new nodes from: https://github.com/dmlc/gluon-nlp/blob/3fbe9619d9e68bc665f73c8cdf683213c6edd4d6/scripts/bert/bertpass_gpu.cc#L280 to: https://github.com/apache/incubator-mxnet/blob/cc4b8ec68b6ec9dae73046e9c34ac97439efda83/example/extensions/lib_subgraph/subgraph_lib.cc#L315

Node* input = graph->addNode(n->name + "_input", "null");

Mostly just little things like this. But this should be a lot of change for the better.

samskalicky avatar Oct 08 '20 21:10 samskalicky

I think it would be ideal to use preprocessor #if to have bertpass_gpu.cc support both MX 1.7 and 1.8

leezu avatar Oct 10 '20 18:10 leezu

I think it would be ideal to use preprocessor #if to have bertpass_gpu.cc support both MX 1.7 and 1.8

thanks for checking this. I will make the modifications as suggested to support both MXNet 1.7 & 1.8

MoisesHer avatar Oct 12 '20 16:10 MoisesHer