-
Notifications
You must be signed in to change notification settings - Fork 912
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
Fix failing xgboost test in the cudf.pandas third-party integration tests #17616
base: branch-25.02
Are you sure you want to change the base?
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test |
# After https://github.com/dmlc/xgboost/pull/11014, .inplace_predict() | ||
# returns a real cupy array when called on a cudf.pandas proxy dataframe. | ||
# So we need to ensure we have a valid numpy array. | ||
# TODO: We should remove the call to .get() when .inplace_predict() |
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 this TODO something we need to change in cudf.pandas, or is it a bug in xgboost?
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.
We'd need to make a change in xgboost. Let me cc @galipremsagar to get his thoughts. I believe .inplace_predict()
should return a cudf.pandas proxy ndarray.
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.
Related issue: #17524
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 don't think we can make .inplace_predict()
return a cudf.pandas
proxy object. Xgboost takes different code-paths for cudf
& pandas
objects, for a library that executes object(gpu/cpu) aware, I don't think there is much we can do rather than the current state of fixes present in xgboost. But I also feel predict
has very old code and it needs to be updated to match inplace_predict
behavior. So I opened a pr asking for reviews: dmlc/xgboost#11118
Co-authored-by: Bradley Dice <[email protected]>
@@ -39,6 +39,7 @@ jobs: | |||
- unit-tests-cudf-pandas | |||
- pandas-tests | |||
- pandas-tests-diff | |||
- third-party-integration-tests-cudf-pandas |
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.
- third-party-integration-tests-cudf-pandas | |
# TODO: Remove this CI job after https://github.com/rapidsai/cudf/issues/17490 is resolved | |
# - third-party-integration-tests-cudf-pandas |
python/cudf/cudf_pandas_tests/third_party_integration_tests/tests/test_xgboost.py
Outdated
Show resolved
Hide resolved
…sts/test_xgboost.py
Description
Apart of #17490
Checklist