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

Add back xgboost.rabit in 1.7 #8407

Closed
wants to merge 2 commits into from

Conversation

hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Nov 1, 2022

Closes #8406 by making xgboost.rabit available in 1.7. Also add a warning to direct users to use xgboost.collective instead.

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 1, 2022

@trivialfis @rongou Can I get a review for this? One downstream project, modin, uses xgboost.rabit in their code.

@hcho3 hcho3 added the Blocking label Nov 1, 2022
@rongou
Copy link
Contributor

rongou commented Nov 1, 2022

@hcho3 the problem is this won't work for xgboost any more since the c++ code depends on the communicators. If we have to keep the existing api, one option is to delegate everything to the xgboost.collective module.

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 1, 2022

@rongou Doesn't xgboost.collective fall down to Rabit in the default setting? For example, xgboost.collective.init() calls rabit::Init internally.

@rongou
Copy link
Contributor

rongou commented Nov 1, 2022

Yes but it needs to set up the RabitCommunicator.

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 1, 2022

@rongou I see. Do you have bandwidth to set up the delegation to xgboost.collective? I can give it a try, but I'm afraid I'd get it wrong, given the substantial difference between xgboost.collective.init (takes in dictionary) and xgboost.rabit.init (takes in list of strings)

@rongou
Copy link
Contributor

rongou commented Nov 1, 2022

Ok I'll take a look.

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 1, 2022

Thanks! Let me know if there's anything I can help. Also, feel free to file a new pull request when you're done.

@rongou
Copy link
Contributor

rongou commented Nov 1, 2022

Do we just want to add back init/finalize to fix modin, or do you want to add all other functions too?

@hcho3
Copy link
Collaborator Author

hcho3 commented Nov 1, 2022

@rongou Let's try to add all functions.

@hcho3 hcho3 closed this Nov 2, 2022
@hcho3 hcho3 deleted the rabit_compat_shim branch November 2, 2022 14:57
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.

2 participants