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.

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

Add support for XGBoost UBJSON in FIL #6009

Merged
merged 4 commits into from
Aug 6, 2024

Conversation

hcho3
Copy link
Contributor

@hcho3 hcho3 commented Aug 5, 2024

Closes #6008

@hcho3 hcho3 requested a review from a team as a code owner August 5, 2024 23:35
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Aug 5, 2024
@hcho3 hcho3 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 5, 2024
@hcho3 hcho3 requested a review from a team as a code owner August 5, 2024 23:42
@hcho3 hcho3 requested a review from KyleFromNVIDIA August 5, 2024 23:42
Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

Just tested and this fixes things perfectly for me

Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Great, thanks! I did not try re-running the notebook yet, but I trust that this works.

Won't block the PR over this since this is time-sensitive for the release... but I think we should have some unit tests covering these new if-else branches, to detect such issues earlier in the future. I'd be happy to do that on 24.10 if you'd like, would be a good opportunity for me to learn a little more about FIL.

@jameslamb
Copy link
Member

Oh wait.... doesn't the notebook need to be updated?

Here it's saying model_type="xgboost"

" model_type='xgboost'\n",

But I think rapidsai/docker CI is going to still be broken, because it'll pull in xgboost=2.1.* and with that, the appropriate model format is "xgboost_ubjson".

cross-ref: rapidsai/docker#699

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@raydouglass raydouglass merged commit efb87b4 into rapidsai:branch-24.08 Aug 6, 2024
26 of 27 checks passed
@hcho3 hcho3 deleted the fil_ubjson_support_2408 branch August 6, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants