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
JNI wrapper for the collective communicator #8242
Conversation
0ba2f3f
to
8312176
Compare
Started buildkite. |
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.
Looks good to me overall. Please document its experimental status.
Added a note about the api being experimental. |
Restarted failed tests. |
Looks like Communicator.java is quite the same as Rabit.java. So does Communicator is dedicated to federated learning? Can it be replaced with the existing Rabit? Looks like we need to re-design this part to abstract the common parts. But that can be in a following-up PR. |
@wbo4958 yes it's more a less a drop-in replacement for Rabit. My next step is to replace all the rabit calls with communicator, and then delete Rabit.java. These changes don't affect the JVM side in terms of behavior, but are needed since we are changing the c++ code. |
@trivialfis can this be merged? Thanks! |
We're probably not going to support federated learning in Spark, but since we'll switch the c++ code to use the communicator interface, this is also needed to keep the java/scala code functioning.