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


   > I think this would be much cleaner if it was a separate directory because:
   > 
   > * Version control is much easier.  Just update mxnet or `rm -rf` it 
without extra cruft
   > * This reflects reality much better.  Somebody else builds mxnet for `pip` 
without knowing about my stuff. MXNet ships a docker in which I can build my 
thing for binary compatibility.
   > * MXNet should be usable as a submodule.
   > 
   > So my ideal instructions look more like
   > 
   > 1. compile MXNet normally from a clean checkout
   > 2. cd into your own project, configure with `-DMXNET=/path/to/mxnet` and 
compile.  Sample project provided and part of integration tests.
   > 3. `my_op.so` built by my build system
   > 4. dynamically load shared library into MXNet via `mx.library.load()`
   
   Agreed, if only MXNet codebase was better organized. We have 
headers/includes scattered throughout the codebase. Not just in 
**include/mxnet**. For example `mxnet_op.h` in in **src/operator** and 
[`include/mshadow/base.h` includes 
`mkl_blas.h`](https://github.com/apache/incubator-mxnet/blob/2610c10701c2b8155dbf094aaecba37ebbf67d0f/3rdparty/mshadow/mshadow/base.h#L173)
 which isnt in the MXNet codebase. Duplicating and reproducing the MXNet cmake 
flow for building custom operators is not worth the hassle/maintenance. 
   
   If others have ideas that they wanna give it a whirl, feel free to checkout 
this branch and try building `min_ex.cc` in the `lib_external_ops` directory. 
Happy to collaborate on this. 
   
   I added a `test` target in the Makefile in `lib_external_ops` to try and 
compile/link as you suggest:
   
https://github.com/apache/incubator-mxnet/pull/18904/files#diff-8a8d486c6b362b11bec05cdd67b3c3bdR32-R33
   But currently its failing with:
   ```
   In file included from ../../../include/mshadow/tensor.h:35:0,
                    from ../../../include/mxnet/base.h:33,
                    from ../../../src/operator/mxnet_op.h:30,
                    from min_ex-inl.h:26,
                    from min_ex.cc:20:
   ../../../include/mshadow/./base.h:173:12: fatal error: mkl_blas.h: No such 
file or directory
      #include <mkl_blas.h>
               ^~~~~~~~~~~~
   ```
   Im happy to be wrong, but I think this is an indication of more to come in 
terms of managing a complex set of includes/dependencies that will constantly 
be changing. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to