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

make lightgbm raw classification predictions backward and forward compatible #42

Merged
merged 1 commit into from
Jul 19, 2022
Merged

make lightgbm raw classification predictions backward and forward compatible #42

merged 1 commit into from
Jul 19, 2022

Conversation

jameslamb
Copy link
Contributor

Contributes to #34.

As of microsoft/LightGBM#5133, lightgbm:::predict.lgb.Booster() no longer supports keyword arguments rawscore, predcontrib, or predleaf.

Those were replaced by equivalent values to the type argument, for consistency with how most statistical models in R work (from both the standard library and 3rd-party packages).

bst <- lgb.train(...)

# before
predict(bst, X, rawscore = TRUE)

# after
predict(bst, X, type = "raw")

Because of the current state of development in LightGBM, we made the difficult choice to make this change in a non-backwards-compatible way, without a deprecation cycle (microsoft/LightGBM#5133 (review)).

{bonsai} provides support for generating raw-score predictions from {lightgbm} models, but that functionality isn't currently covered by tests.

Checked with {covr}.

Rscript \
    --vanilla \
    -e "covr::report(covr::package_coverage(), file = file.path(getwd(), 'coverage.html'))"

image

This PR proposes adding a small patch to make {bonsai} compatible with previous and future releases of {lightgbm}, so that the eventual release of {lightgbm} v4.0.0 will hopefully not be disruptive.

It also adds unit tests generating raw predictions for classification models, to cover this functionality and increase the chance of catching future incompatibilities.

How I tested this

Tested against the development version of {lightgbm} following using the approach described in #34 (comment).

Notes for Reviewers

The unit tests I've added are just enough to cover the relevant code and minimally test that it's producing expected results. If you'd prefer stricter tests, please let me know and I'd be happy to add them.

Thanks very much for your time and consideration.

@jameslamb jameslamb changed the title make lightgbm raw classification predictions back and forwards compatible make lightgbm raw classification predictions backward and forward compatible Jul 19, 2022
Copy link
Contributor

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

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

This looks super clean, thanks @jameslamb! The context is much appreciated, too.

Will go ahead and merge.🚀

@simonpcouch simonpcouch merged commit 5b32d8d into tidymodels:main Jul 19, 2022
@jameslamb
Copy link
Contributor Author

Thanks very much!

@jameslamb jameslamb deleted the fix/lightgbm-raw-score branch July 19, 2022 14:21
@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants