-
-
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
support multiple batches in gpu_hist #5014
Changes from 23 commits
4e0824e
f5160cc
397301e
ca7f132
57121bf
aae1e8f
48fabd6
a299db1
4365625
f9f0b7e
20fef95
0b48ab6
bb6afd8
3ead65d
f0f8b54
68d08c5
c6b8e8a
70e424e
a6cca35
8d8b426
14734d9
2a3c02a
e86edc1
5a75451
0662f8d
a894c36
73eec1a
3b716ce
377d043
a6bf0dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ struct GenericParameter : public XGBoostParameter<GenericParameter> { | |
int nthread; | ||
// primary device, -1 means no gpu. | ||
int gpu_id; | ||
// gpu page size in external memory mode, 0 means using the default. | ||
size_t gpu_page_size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parameter is configurable by users, please don't define it twice. Make one of them a normal variable. If we don't want to configure it by user, don't use parameter at all. As pickling might loss some of these information, dask uses pickle to move booster around workers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's only defined as a configurable parameter once here, the other one is really just plumbing. For now this is mostly used for testing, but perhaps user may want to set it depending on the GPU memory they have. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Em.. we want to do parameter validation, like detecting unused parameters. This may add some extra difficulties. Do you think it's possible to set it as a DMatrix parameter instead of a global one? Maybe another PR? Sorry for nitpicking here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, we need to be careful adding global parameters due to upcoming work on serialisation. Unless you see a strong motivation for users tuning this, let's leave it out for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not quite sure if this is useful for end users. Is there way to make a parameter hidden/internal? It's really useful for the tests since we don't have to build a dataset bigger than 32MB. |
||
|
||
void CheckDeprecated() { | ||
if (this->n_gpus != 0) { | ||
|
@@ -49,6 +51,10 @@ struct GenericParameter : public XGBoostParameter<GenericParameter> { | |
.set_default(-1) | ||
.set_lower_bound(-1) | ||
.describe("The primary GPU device ordinal."); | ||
DMLC_DECLARE_FIELD(gpu_page_size) | ||
.set_default(0) | ||
.set_lower_bound(0) | ||
.describe("GPU page size when running in external memory mode."); | ||
DMLC_DECLARE_FIELD(n_gpus) | ||
.set_default(0) | ||
.set_range(0, 1) | ||
|
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.
Do we we expose this to users?
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.
Yes. See below.