Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

bertpass_gpu.cc does not support MXNet 1.8 #1388

Open
leezu opened this issue Oct 8, 2020 · 3 comments
Open

bertpass_gpu.cc does not support MXNet 1.8 #1388

leezu opened this issue Oct 8, 2020 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@leezu
Copy link
Contributor

leezu commented Oct 8, 2020

Due to API change apache/mxnet#19135

@leezu leezu added the bug Something isn't working label Oct 8, 2020
@samskalicky
Copy link

samskalicky commented Oct 8, 2020

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

MXReturnValue custom_pass(const std::string& in_graph, const std::string** out_graph,
const std::unordered_map<std::string, std::string>& options,
const std::unordered_map<std::string, MXTensor>& args,
const std::unordered_map<std::string, MXTensor>& aux,
const PassResource& res) {
for (auto kv : options)
std::cout << "option: " << kv.first << " ==> " << kv.second << std::endl;
//convert graph from JSON string to Graph/Node data structure
Graph g = Graph::fromString(in_graph);
//g.print();

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:

for(Node* n : g.nodes){

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:

Node* node_expand_1_bias = new Node();

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.

@leezu
Copy link
Contributor Author

leezu commented Oct 10, 2020

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

@MoisesHer
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants