Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MXNet: use MXEnginePushAsync C API to push horovod operations #985

Merged
merged 6 commits into from Apr 19, 2019

Conversation

4 participants
@yuxihu
Copy link
Contributor

commented Apr 4, 2019

Fixes #884

This PR changes PushAsync to MXEnginePushAsync C API to avoid std::function incompatibility between GCC 4.x and GCC 5.x.

The dependent PR in MXNet has been merged.

@yuxihu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

@eric-haibin-lin @ctcyang @apeforest Please help review

Show resolved Hide resolved horovod/mxnet/mpi_ops.h Outdated

@yuxihu yuxihu force-pushed the yuxihu:mx_func_ptr branch from c9413a6 to 468aa93 Apr 9, 2019

@alsrgv

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2019

@yuxihu, please note that there is another tf-nightly build break that I just pushed workaround for in the master, so please rebase.

@yuxihu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@alsrgv Thanks for letting me know. I will do it later once the dependent PR is merged into MXNet. I will need to update the MXNet package in order to use the new API so it will fail anyway right now.

@yuxihu yuxihu changed the title MXNet: use PushAsyncPtr interface to push horovod operations MXNet: use MXEnginePushAsync C API to push horovod operations Apr 12, 2019

yuxihu added some commits Apr 8, 2019

use PushAsyncPtr API
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>
refactor code
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>
sync with API changes
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>
fix opr_name
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>

@yuxihu yuxihu force-pushed the yuxihu:mx_func_ptr branch 2 times, most recently from 4190e1f to 48520ec Apr 15, 2019

update MXNet package in CI
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>

@yuxihu yuxihu force-pushed the yuxihu:mx_func_ptr branch from 48520ec to f7b31e0 Apr 15, 2019

@yuxihu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

@apeforest @alsrgv The PR is ready for review.

address comments
Signed-off-by: Yuxi Hu <darrenyxhu@gmail.com>
@apeforest
Copy link
Contributor

left a comment

LGTM

@yuxihu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

@alsrgv Could you please take a look?

A side QQ: when will be the next Horovod release?

@alsrgv

alsrgv approved these changes Apr 19, 2019

Copy link
Collaborator

left a comment

Sorry for the delayed response - I'm in vacation.

LGTM, thanks! I think one future idea to explore would be a header only C++ interface that would translate these C APIs the to C++ without imposing ABI compatibility issues.

Re release - next week when I'm back.

@alsrgv alsrgv merged commit 98d4489 into horovod:master Apr 19, 2019

2 checks passed

DCO DCO
Details
buildkite/horovod/pr Build #149 passed (43 minutes, 33 seconds)
Details

@yuxihu yuxihu deleted the yuxihu:mx_func_ptr branch Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.