Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] Add scheduler address to dask config. #7581
[dask] Add scheduler address to dask config. #7581
Changes from 10 commits
cff0733
137bd95
50d701d
c51eabe
ca332d0
b33559d
1c9f466
d8d7ccb
6e97a65
7a87d21
16bf957
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
When extending the Dask config system we typically create a new YAML with default values in and extend the Dask config with it.
See how we do it in dask-ctl.
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 sharing, let me look into that.
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'm curious why this
dconfig
object is being passed around instead of accessingdask.config.get("xgboost.scheduler_address")
directly.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 couldn't access it on the worker side for some reason (key not found). I read the doc from https://docs.dask.org/en/stable/configuration.html , it's not quite clear to me how to make sure everything is synced.
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 also read this section https://docs.dask.org/en/stable/configuration.html#downstream-libraries . In XGBoost dask is loaded lazily so having dask in init might be less desirable due to the size of dependency. So I went on and read this:
So far using this parameter passing approach seems to be the easiest way to get 1 parameter in.
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.
That makes sense. Generally the Dask config should be available on the scheduler/workers but that can be deployment dependent.