[GitHub] [incubator-mxnet] samskalicky edited a comment on pull request #18904: MXNet external operators

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

samskalicky edited a comment on pull request #18904:
URL: https://github.com/apache/incubator-mxnet/pull/18904#issuecomment-680613651


   Dear community members,
   
   After further analysis and help from many people, I have decided that the best way forward is to put this work on hold while we spend some more time improving the C++ APIs of both MXNet and our dependencies (nnvm, dmlc-core). Once we have a stable, maintainable, versioned set of C++ APIs and a consistent process to build and link libmxnet.so with external libraries we'll revisit this proposal. 
   
   The major problem we ran into was that we couldnt decide to expose all symbols in libmxnet.so, so to be able to link a custom library with external operators in it we need to export only the specific symbols needed to build an operator (or other external components). And some of these symbols are coming from our dependencies like NNVM (ie. `NNVM_REGISTER_OP`). So we will need to ensure these symbols are exported from those projects as well. This will mean building a libnnvm.so and linking it with libmxnet.so so that it can be more easily maintained. Custom libraries will then be able to link against libnnvm.so and  libmxnet.so. 
   
   The other big problem was that we found that in order to compile an external operator you really had to use the main MXNet CMakeLists.txt to generate all the defines/includes/etc so that you can compile your library correctly. This was a huge pain, and ideally we would have a small set of includes that are opaque (ie. dont further include all of MXNet's includes) to use instead. 
   
   So we'll work towards both of these goals (symbol handling and includes) to reconfigure MXNet and its dependencies to have a cleaner C++ API. Once we have that then it will be easier to dynamically load libraries with external ops. 
   
   In the meantime, some of the refactoring work will continue on in another PR #19016. I will close this PR for now. Thanks for all of your input on this proposal!
   
   Sam


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