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

Fix handling of global bias for binary:logitraw objective of XGBoost #242

Merged
merged 1 commit into from Dec 29, 2020

Conversation

hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Dec 29, 2020

Prior to dmlc/xgboost#6517, XGBoost was incorrectly applying the inverse sigmoid function the global bias when the objective was set to binary:logitraw. (The correct action is to apply no transformation.) Treelite was mimicking XGBoost's behavior.

XGBoost 1.3.1 and later has dmlc/xgboost#6517 and thus will correctly handle the global bias, so update Treelite as well.

@hcho3 hcho3 mentioned this pull request Dec 29, 2020
8 tasks
@codecov-io
Copy link

Codecov Report

Merging #242 (2d0549b) into mainline (191a474) will increase coverage by 65.78%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##             mainline     #242       +/-   ##
===============================================
+ Coverage       17.93%   83.72%   +65.78%     
  Complexity         44       44               
===============================================
  Files              65       93       +28     
  Lines            4555     6925     +2370     
  Branches           42       42               
===============================================
+ Hits              817     5798     +4981     
+ Misses           3714     1103     -2611     
  Partials           24       24               
Impacted Files Coverage Δ Complexity Δ
src/frontend/xgboost/xgboost.h 100.00% <ø> (+100.00%) 0.00 <0.00> (ø)
src/frontend/xgboost.cc 79.70% <100.00%> (+79.70%) 0.00 <0.00> (ø)
src/frontend/xgboost_json.cc 69.60% <100.00%> (+69.60%) 0.00 <0.00> (ø)
src/frontend/xgboost_util.cc 97.43% <100.00%> (+97.43%) 0.00 <0.00> (ø)
src/predictor/thread_pool/spsc_queue.h 93.87% <0.00%> (-0.72%) 0.00% <0.00%> (ø%)
src/logging.cc 100.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
src/typeinfo.cc 100.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
python/treelite/sklearn/__init__.py 90.90% <0.00%> (ø) 0.00% <0.00%> (?%)
python/treelite/sklearn/gbm_multi_classifier.py 94.11% <0.00%> (ø) 0.00% <0.00%> (?%)
python/treelite/contrib/gcc.py 100.00% <0.00%> (ø) 0.00% <0.00%> (?%)
... and 74 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 191a474...2d0549b. Read the comment docs.

@hcho3 hcho3 merged commit b6c9d39 into dmlc:mainline Dec 29, 2020
@hcho3 hcho3 deleted the fix_logitraw branch December 29, 2020 12:28
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request Jan 7, 2021
Depends on dmlc/treelite#235, dmlc/treelite#236, dmlc/treelite#237, dmlc/treelite#240, dmlc/treelite#241, dmlc/treelite#242, dmlc/treelite#243

Depends on rapidsai/integration#201

Closes #2160
Closes #2795
Closes #2817

TODOs
- [x] Get code review for dmlc/treelite#235 and dmlc/treelite#236 and merge them
- [x]  Get code review for dmlc/treelite#237 and merge them
- [x] Merge dmlc/treelite#240, dmlc/treelite#241, dmlc/treelite#242, dmlc/treelite#243
- [x] Run tests locally
- [x] Tag a new version of Treelite
- [x] Release a new Conda package with the new version tag
- [x] Update `meta.yml` and the integration package to use the new Treelite
- [x] Run tests (this time build with new Conda package)

Authors:
  - Hyunsu Cho <chohyu01@cs.washington.edu>

Approvers:
  - AJ Schmidt (@ajschmidt8)
  - William Hicks (@wphicks)
  - John Zedlewski (@JohnZed)

URL: #3316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants