Skip to content

Make implementation of GrpcServer::Init with Collective Ops compatible with calls in contrib/mpi#19942

Merged
caisq merged 3 commits intotensorflow:masterfrom
emaballarin:master
Jun 13, 2018
Merged

Make implementation of GrpcServer::Init with Collective Ops compatible with calls in contrib/mpi#19942
caisq merged 3 commits intotensorflow:masterfrom
emaballarin:master

Conversation

@emaballarin
Copy link
Copy Markdown
Contributor

Fixes compilation errors outlined in #19924.

I haven't tested the actual MPI behavior (just that compilation succeeds). However, an if-clause in grpc_server_lib.cc guards the case in which collective_mgr_func is not explicitly passed.

@caisq caisq requested a review from mrry June 12, 2018 13:19
@caisq caisq self-assigned this Jun 12, 2018
@caisq caisq added awaiting review Pull request awaiting review kokoro:force-run Tests on submitted change labels Jun 12, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 12, 2018
@mrry mrry requested review from poxvoculi and removed request for mrry June 12, 2018 14:06
poxvoculi
poxvoculi previously approved these changes Jun 12, 2018
@emaballarin
Copy link
Copy Markdown
Contributor Author

Sorry for the additional noise (and additional commit). Having to compile TF with GCC 8.1/OpenMPI 3.1, and thus having to patch some MPI-related code to make it compile, I ported the changes for the PR by hand. Indeed, a closing brace was deleted together with the argument. This (probably) explains the CI tests failures. I'm very sorry for that. Now it should be fixed.

Copy link
Copy Markdown
Contributor

@poxvoculi poxvoculi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR

@caisq caisq added kokoro:force-run Tests on submitted change awaiting testing (then merge) and removed awaiting review Pull request awaiting review labels Jun 12, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 12, 2018
@caisq
Copy link
Copy Markdown
Contributor

caisq commented Jun 13, 2018

It appears that there is some tool failure in the "MacOS Python2 and CC" kokoro test. Starting test again.

@caisq caisq added the kokoro:force-run Tests on submitted change label Jun 13, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 13, 2018
@caisq caisq merged commit 4b5f4a5 into tensorflow:master Jun 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants