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
Define the new
device
parameter. #9362Define the new
device
parameter. #9362Changes from 10 commits
ff12bad
26d02c3
887c1c4
79b88ff
8119408
2a49e07
eb7a17b
15b71cc
49f1e91
00e39b1
63e632c
9260e6a
ebbb852
99a6a48
dea8a75
1d91534
46add85
5e8f7f0
45454b0
871dcbc
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.
I understand that now this are equivalent from XGBoost perspective, but as far we are talking about API used in next many years would it make sense to not introduce this restriction?
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.
Absolutely, that's why we have
gpu
andcuda
. It's equal tocuda
"for now", in the future others can make variants based on available GPU devices at run time.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.
@napetrov Feel free to share your suggestions. ;-) Would like to get more opinions.
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 mean from API perspective users would be writing code assuming GPU device are always CUDA, but this would results in breaking change when there would be another GPU backend.
Might be it worth to point that this is not the same but just a convenient way for selecting default GPU device - so in long-term this would result in GPU dispatching regardless of particular HW. i.e. now this is not same as 'cuda' but is default GPU device selector although it can select only from ['cuda']. In this way we would clearly define expectations from API and would allow extensions here
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, will change the document as suggested.
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.
Something along this lines
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.
Done.