Skip to content

Handle LightGBM models with '=' in feature names - #298

Merged
chyunsu3 merged 3 commits into
dmlc:mainlinefrom
chyunsu3:handle_feat_name_with_eq_sign
Jul 15, 2021
Merged

Handle LightGBM models with '=' in feature names#298
chyunsu3 merged 3 commits into
dmlc:mainlinefrom
chyunsu3:handle_feat_name_with_eq_sign

Conversation

@chyunsu3

Copy link
Copy Markdown
Collaborator

Closes #281

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This test case creates a Pandas dataframe with '=' in the column names.

@codecov

codecov Bot commented Jul 13, 2021

Copy link
Copy Markdown

Codecov Report

Merging #298 (9f67339) into mainline (d037bb4) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff               @@
##             mainline     #298      +/-   ##
==============================================
- Coverage       84.97%   84.75%   -0.23%     
  Complexity         46       46              
==============================================
  Files             106      104       -2     
  Lines            8034     7019    -1015     
  Branches           50       50              
==============================================
- Hits             6827     5949     -878     
+ Misses           1182     1045     -137     
  Partials           25       25              
Impacted Files Coverage Δ
src/frontend/lightgbm.cc 87.42% <100.00%> (+2.10%) ⬆️
python/treelite/libpath.py 75.00% <0.00%> (-7.15%) ⬇️
runtime/python/treelite_runtime/libpath.py 75.00% <0.00%> (-7.15%) ⬇️
src/frontend/xgboost/xgboost_json.h 83.07% <0.00%> (-6.14%) ⬇️
include/treelite/base.h 47.05% <0.00%> (-5.58%) ⬇️
src/compiler/native/typeinfo_ctypes.h 43.47% <0.00%> (-4.53%) ⬇️
src/compiler/ast/ast.h 34.92% <0.00%> (-3.91%) ⬇️
src/gtil/predict.cc 81.81% <0.00%> (-3.69%) ⬇️
src/typeinfo.cc 75.00% <0.00%> (-2.78%) ⬇️
runtime/python/treelite_runtime/util.py 92.10% <0.00%> (-2.64%) ⬇️
... and 55 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 d037bb4...9f67339. Read the comment docs.

@chyunsu3
chyunsu3 force-pushed the handle_feat_name_with_eq_sign branch from 0fe9cad to 80dd7a7 Compare July 13, 2021 23:43
Comment thread src/frontend/lightgbm.cc
Comment on lines +246 to +249
if (!rest.empty()) {
value += "=";
value += rest;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This change lets us handle the line

feature_names=x0=0 x0=1 x1=0 x1=1 x1=2

in model.txt.

@chyunsu3

Copy link
Copy Markdown
Collaborator Author

@trivialfis Can you take a quick look and see if the test case looks reasonable?

@trivialfis trivialfis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test looks good. Maybe we can add a check to see if the generated model does have such feature names to be sure that we are testing the right thing.

@chyunsu3
chyunsu3 merged commit b2f52bd into dmlc:mainline Jul 15, 2021
@chyunsu3
chyunsu3 deleted the handle_feat_name_with_eq_sign branch July 15, 2021 00:53
This was referenced Jul 16, 2021
rapids-bot Bot pushed a commit to NVIDIA/cuml that referenced this pull request Jul 21, 2021
Closes #4037.

The 2.0.0 version of Treelite incorporates the following major improvements:

* **Remove dmlc-core dependency** (dmlc/treelite#284).
* Rename CHECK and LOG macros to avoid name collisions (dmlc/treelite#295)
* Suppress warning about build time when parallel_comp is specified (dmlc/treelite#297)
* Handle LightGBM models with '=' in feature names (dmlc/treelite#298)
* Remove OpenMP dependency (dmlc/treelite#300)

Depends on rapidsai/integration#315

Authors:
  - Philip Hyunsu Cho (https://github.com/hcho3)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Jordan Jacobelli (https://github.com/Ethyling)

URL: #4072
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
Closes NVIDIA#4037.

The 2.0.0 version of Treelite incorporates the following major improvements:

* **Remove dmlc-core dependency** (dmlc/treelite#284).
* Rename CHECK and LOG macros to avoid name collisions (dmlc/treelite#295)
* Suppress warning about build time when parallel_comp is specified (dmlc/treelite#297)
* Handle LightGBM models with '=' in feature names (dmlc/treelite#298)
* Remove OpenMP dependency (dmlc/treelite#300)

Depends on rapidsai/integration#315

Authors:
  - Philip Hyunsu Cho (https://github.com/hcho3)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Jordan Jacobelli (https://github.com/Ethyling)

URL: NVIDIA#4072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ill-formed LightGBM model file when the model is fitted from a pd.DataFrame having the character '=' inside any of column name.

2 participants