Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update FIL tests to use XGBoost UBJSON instead of binary #6153

Merged
merged 6 commits into from
Dec 4, 2024
Merged

Update FIL tests to use XGBoost UBJSON instead of binary #6153

merged 6 commits into from
Dec 4, 2024

Conversation

hcho3
Copy link
Contributor

@hcho3 hcho3 commented Dec 3, 2024 •
edited
Loading

Starting from 2.1, XGBoost uses UBJSON format to serialize models.
Replace all uses of the legacy model format with UBJSON.

Also make xgboost a test dependency of cuML so that the FIL tests run in the CI pipelines.

@hcho3 hcho3 requested a review from a team as a code owner December 3, 2024 01:03
@hcho3 hcho3 requested review from cjnolet and wphicks December 3, 2024 01:03
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Dec 3, 2024
@hcho3 hcho3 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 3, 2024
@hcho3 hcho3 requested a review from a team as a code owner December 3, 2024 01:37
@hcho3 hcho3 requested a review from bdice December 3, 2024 01:37
@github-actions github-actions bot added the conda conda issue label Dec 3, 2024
robertmaynard
robertmaynard reviewed Dec 3, 2024
View reviewed changes
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we cant use xgboost>=2.1.3 ?

jakirkham
jakirkham requested changes Dec 3, 2024
View reviewed changes
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Hyunsu! 🙏

Had a recommendation below

dependencies.yaml Outdated Show resolved Hide resolved
jakirkham
jakirkham approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks Hyunsu! 🙏

@jakirkham
Copy link
Member

Checked in offline. Sounds like we are ready to merge this. Going to queue it to merge so we don't forget (as code freeze is around the corner)

@jakirkham
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 53259f5 into rapidsai:branch-24.12 Dec 4, 2024
65 checks passed
@hcho3 hcho3 deleted the update_fil_tests branch December 4, 2024 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wphicks wphicks wphicks approved these changes

@jakirkham jakirkham jakirkham approved these changes

@robertmaynard robertmaynard robertmaynard approved these changes

@cjnolet cjnolet Awaiting requested review from cjnolet cjnolet is a code owner automatically assigned from rapidsai/cuml-python-codeowners

@bdice bdice Awaiting requested review from bdice bdice is a code owner automatically assigned from rapidsai/packaging-codeowners

Assignees

@hcho3 hcho3

@jakirkham jakirkham

Labels
conda conda issue Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hcho3 @jakirkham @robertmaynard @wphicks