-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[dask] allow parameter aliases for local_listen_port, num_threads, tree_learner (fixes #3671) #3789
Conversation
Hmmm, it doesn't look like GPU error is related to these Dask changes:
|
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.
Thanks a lot for implementing my feature request! 🙂
Could please do the same with aliases here?
LightGBM/python-package/lightgbm/basic.py
Lines 2078 to 2081 in 4007b34
self.set_network(machines, | |
local_listen_port=params.get("local_listen_port", 12400), | |
listen_time_out=params.get("listen_time_out", 120), | |
num_machines=params.setdefault("num_machines", num_machines)) |
I think it OK to use the same PR for implementing network aliases in different placed of codebase. But feel free to disagree with me and say that it requires a separate PR or/and feature request.
Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
I'm happy to do this soon, but I'd like to do it as a separate PR. I have a preference for keeping the dask PRs small and quick, to prevent too much pain for @ffineis on #3708, and to make sure that the Dask interface keeps progressing towards something we're comfortable releasing with 3.2.0. You only get one chance to release code as "new" 😀 |
Looks like Dask tests are quite flaky:
Sure, thanks! 👍 |
I'm realizing that too 😫 . @ffineis faced the same thing on #3708, and I think he's come up with a good solution for now on that PR. Also |
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.
Great, thanks!
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
This PR closes #3671. It updates the code in
lightgbm.dask
so that it respects parameter aliases.So, for example, as of this PR you can use
tree_learner
,tree
,tree_type
, ortree_learner_type
to refer to thetree_learner
parameter that controls which type of distributed training is performed.