-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-1110] Add header files required by horovod #13062
[MXNET-1110] Add header files required by horovod #13062
Conversation
@szha Your review and advice are very much appreciated. |
Do we need the headers for the submodules too? That would also become really hard to maintain. |
@apeforest Thanks for your contribution! |
@rahul003 Yes, because mxnet/engine.h --> mxnet/base.h --> All these submodules. |
@apeforest thanks for your contribution. Could you please update your branch and re-trigger CI? @mxnet-label-bot update [pr-awaiting-review] |
@mxnet-label-bot update [pr-awaiting-review] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. How are you verifying this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we address the other open questions in the review.
@rahul003 Yes, all the added header files are required by Horovod. |
@szha Yes, we have verified that Horovod works well with MXNet with these included header files in pip package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@mxnet-label-bot update [pr-awaiting-merge] |
@anirudh2290 Can this PR be merged now? |
* Add header files required by horovod * Add symbolic link and cherry picked required header * add python API to return include path * update link * fix windows CI * fix windows build * fix dlpack link * merge with master * exclude 3rd party header files from license check * exclude license check * exclude include directory * remove commented lines
* Add header files required by horovod * Add symbolic link and cherry picked required header * add python API to return include path * update link * fix windows CI * fix windows build * fix dlpack link * merge with master * exclude 3rd party header files from license check * exclude license check * exclude include directory * remove commented lines
* Add header files required by horovod * Add symbolic link and cherry picked required header * add python API to return include path * update link * fix windows CI * fix windows build * fix dlpack link * merge with master * exclude 3rd party header files from license check * exclude license check * exclude include directory * remove commented lines
* Add header files required by horovod * Add symbolic link and cherry picked required header * add python API to return include path * update link * fix windows CI * fix windows build * fix dlpack link * merge with master * exclude 3rd party header files from license check * exclude license check * exclude include directory * remove commented lines
* Add header files required by horovod * Add symbolic link and cherry picked required header * add python API to return include path * update link * fix windows CI * fix windows build * fix dlpack link * merge with master * exclude 3rd party header files from license check * exclude license check * exclude include directory * remove commented lines
Description
When integrating mxnet with horovod, the callback functions need to use certain MXNet APIs which are not currently explosed in include directory. This PR adds the required header files to the include directory as soft links. The header files are already exported in the pip package and verified on Horovod side.