-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
[pyspark] Support collective.Conf in spark #11003
Conversation
python-package/xgboost/spark/core.py
Outdated
@@ -257,21 +258,20 @@ class _SparkXGBParams( | |||
"launched on the driver side; otherwise, it will be launched on the executor side.", | |||
TypeConverters.toBoolean, | |||
) | |||
tracker_host_ip = Param( | |||
tracker = Param( |
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.
the name "tracker" is ok?
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.
I would probably continue to use the name coll_cfg
just to be consistent. Besides, the configuration is for both the tracker and the communicators.
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.
sure, I changed it to collective_conf
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.
I know everyone has a naming preference ... if you use a new name, please modify the existing ones in Dask for consistency.
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.
OK, I just changed it to coll_cfg to be compatible with dask.
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.
Thank you for the PR!
This PR is a followup of #10983