-
-
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
Remove internal use of gpu_id. #9568
Conversation
Why are we dropping gtest from the CI? |
@hcho3 windows build is failing with cmake find gtest. (same error on master branch). Since the test is building the Python package, I figure we don't need to build c++ tests as we are not running it. |
Are we not running gtest on Windows CI? |
only on buildkite. |
Ah, got it |
786b4aa
to
ae18374
Compare
src/common/host_device_vector.cu
Outdated
@@ -138,7 +138,7 @@ class HostDeviceVectorImpl { | |||
} else { | |||
auto ptr = other->ConstDevicePointer(); | |||
SetDevice(); | |||
CHECK_EQ(this->DeviceIdx(), other->DeviceIdx()); | |||
CHECK_EQ(this->Device().ordinal, other->Device().ordinal); |
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.
CHECK_EQ(this->Device().ordinal, other->Device().ordinal); | |
CHECK(this->Device() == other->Device()); |
if (device_.IsCUDA() && device.IsCUDA()) { | ||
CHECK_EQ(device_.ordinal, device.ordinal) |
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.
Is it possible for two DeviceOrd
objects to be different when they have the same positive value in the ordinal
field ?
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.
Not possible at the moment. Unless there's new device type other than CUDA.
src/predictor/gpu_predictor.cu
Outdated
CHECK(!p_fmat->Info().IsColumnSplit()) | ||
<< "Predict contribution support for column-wise data split is not yet implemented."; |
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.
Is SHAP support available for column-wise split data now?
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 check was removed between PR rebases. I will bring it back.
Device communicator is still using integer ordinal, which will be changed by upcoming PRs.