feature importance type in saved model file #3220

guolinke · 2020-07-12T04:11:10Z

No description provided.

StrikerRUS · 2020-07-12T15:31:58Z

@guolinke Don't you want to expose this parameter to C API? With 3.0 I guess it's good time to break some code (if any). Then wrappers will be able to provide this param directly in function like start/end iterations

LightGBM/python-package/lightgbm/basic.py

Lines 2734 to 2750 in 2792923

    
               def dump_model(self, num_iteration=None, start_iteration=0): 
        
                   """Dump Booster to JSON format. 
        
                   Parameters 
        
                   ---------- 
        
                   num_iteration : int or None, optional (default=None) 
        
                       Index of the iteration that should be dumped. 
        
                       If None, if the best iteration exists, it is dumped; otherwise, all iterations are dumped. 
        
                       If <= 0, all iterations are dumped. 
        
                   start_iteration : int, optional (default=0) 
        
                       Start index of the iteration that should be dumped. 
        
                   Returns 
        
                   ------- 
        
                   json_repr : dict 
        
                       JSON format of Booster. 
        
                   """

guolinke · 2020-07-13T03:39:56Z

Thanks @StrikerRUS , I change all related functions.

jameslamb

I see that the R unit tests are failing. To get better logs, you can do this:

Rscript build_r.R
cd R-package/tests/
Rscript testthat.R

If you don't have an R environment available easily let me know and I can run that for you.

R-package/R/lgb.Booster.R

src/c_api.cpp

Co-authored-by: James Lamb <jaylamb20@gmail.com>

…osoft/LightGBM into feature-importance-type-cli

guolinke · 2020-07-13T04:16:57Z

ping @imatiach-msft for swig fix

imatiach-msft · 2020-07-13T15:05:24Z

@guolinke this looks great!

imatiach-msft

LGTM!

StrikerRUS · 2020-07-13T19:05:21Z

@guolinke In my commit 4f3803d I
replaced cout type with split type according to the existing terminology:

LightGBM/include/LightGBM/c_api.h

Lines 1067 to 1076 in 61b3c30

    
           /*! 
        
            * \brief Get model feature importance. 
        
            * \param handle Handle of booster 
        
            * \param num_iteration Number of iterations for which feature importance is calculated, <= 0 means use all 
        
            * \param importance_type Method of importance calculation: 
        
            *   - 0 for split, result contains numbers of times the feature is used in a model; 
        
            *   - 1 for gain, result contains total gains of splits which use the feature 
        
            * \param[out] out_results Result array with feature importance 
        
            * \return 0 when succeed, -1 when failure happens 
        
            */

LightGBM/src/boosting/gbdt_model_text.cpp

Line 618 in 61b3c30

Log::Fatal("Unknown importance type: only support split=0 and gain=1");

added defines in C API for better user experience and changed in Python package param type from int to string for better usage and to unify with existing codebase:

LightGBM/python-package/lightgbm/sklearn.py

Lines 244 to 247 in 61b3c30

    
                   importance_type : string, optional (default='split') 
        
                       The type of feature importance to be filled into ``feature_importances_``. 
        
                       If 'split', result contains numbers of times the feature is used in a model. 
        
                       If 'gain', result contains total gains of splits which use the feature.

LightGBM/python-package/lightgbm/basic.py

Lines 2956 to 2959 in 61b3c30

    
                   importance_type : string, optional (default="split") 
        
                       How the importance is calculated. 
        
                       If "split", result contains numbers of times the feature is used in a model. 
        
                       If "gain", result contains total gains of splits which use the feature.

StrikerRUS · 2020-07-13T19:19:14Z

include/LightGBM/config.h

+  // desc = **Note**: can be used only in CLI version
+  int saved_feature_importance_type = 0;


@guolinke I added CLI-only flag. Should it be marked as [no-save] as well?

yeah, CLI-only is needed. As for [no-save], I think it is better not marked, as user may want to know which feature importance type is saved in the model file.

OK. Agree with you!

StrikerRUS · 2020-07-13T19:31:47Z

Fixed #3011.

guolinke · 2020-07-14T01:26:59Z

Thank you @StrikerRUS for the doc improvement, as always!

StrikerRUS · 2020-07-14T14:16:36Z

@jameslamb Is something left for R-package?

jameslamb

R side looks good to me! It would be good to have an R test for using gain-based feature importance but I can do that in a later PR

StrikerRUS · 2020-07-15T19:19:38Z

@jameslamb Thank you! I think it's better to have a separate PR for R tests.

github-actions · 2023-08-24T12:19:41Z

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

feature importance type in saved model file

f399900

guolinke requested review from btrotta, chivee, jameslamb, Laurae2 and StrikerRUS as code owners July 12, 2020 04:11

guolinke added the feature label Jul 12, 2020

guolinke and others added 2 commits July 12, 2020 12:26

fix nullptr

d77d054

fixed formatting

4293ed2

fix python/R

557e9c9

guolinke requested review from henry0312 and wxchan as code owners July 13, 2020 03:31

jameslamb reviewed Jul 13, 2020

View reviewed changes

jameslamb requested changes Jul 13, 2020

View reviewed changes

R-package/R/lgb.Booster.R Outdated Show resolved Hide resolved

R-package/R/lgb.Booster.R Outdated Show resolved Hide resolved

R-package/R/lgb.Booster.R Outdated Show resolved Hide resolved

guolinke commented Jul 13, 2020

View reviewed changes

src/c_api.cpp Outdated Show resolved Hide resolved

guolinke and others added 5 commits July 13, 2020 11:58

Update src/c_api.cpp

613886f

Apply suggestions from code review

09f2545

Co-authored-by: James Lamb <jaylamb20@gmail.com>

fix c_api test

4ff47de

Merge branch 'feature-importance-type-cli' of https://github.com/Micr…

a48a00f

…osoft/LightGBM into feature-importance-type-cli

fix swig

9d2033d

imatiach-msft approved these changes Jul 13, 2020

View reviewed changes

Merge branch 'master' into feature-importance-type-cli

28cc2e0

StrikerRUS force-pushed the feature-importance-type-cli branch from b0bea13 to a275533 Compare July 13, 2020 18:59

minor docs improvements and added defines for importance types

4f3803d

StrikerRUS force-pushed the feature-importance-type-cli branch from a275533 to 4f3803d Compare July 13, 2020 19:09

StrikerRUS reviewed Jul 13, 2020

View reviewed changes

jameslamb approved these changes Jul 15, 2020

View reviewed changes

StrikerRUS merged commit 87d4648 into master Jul 15, 2020

StrikerRUS deleted the feature-importance-type-cli branch July 15, 2020 19:20

This was referenced Jul 15, 2020

save_model() Feature Importance Section can use a different importance_type #3011

Closed

added feature importance to JSON dump #2656

Merged

jameslamb mentioned this pull request Jul 16, 2020

[R-package] add test on importance types #3231

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature importance type in saved model file #3220

feature importance type in saved model file #3220

guolinke commented Jul 12, 2020

StrikerRUS commented Jul 12, 2020

guolinke commented Jul 13, 2020

jameslamb left a comment

guolinke commented Jul 13, 2020

imatiach-msft commented Jul 13, 2020

imatiach-msft left a comment

StrikerRUS commented Jul 13, 2020 •

edited

Loading

StrikerRUS Jul 13, 2020

guolinke Jul 14, 2020

StrikerRUS Jul 14, 2020

StrikerRUS commented Jul 13, 2020

guolinke commented Jul 14, 2020

StrikerRUS commented Jul 14, 2020

jameslamb left a comment

StrikerRUS commented Jul 15, 2020

github-actions bot commented Aug 24, 2023

		// desc = Note: can be used only in CLI version
		int saved_feature_importance_type = 0;

feature importance type in saved model file #3220

feature importance type in saved model file #3220

Conversation

guolinke commented Jul 12, 2020

StrikerRUS commented Jul 12, 2020

guolinke commented Jul 13, 2020

jameslamb left a comment

Choose a reason for hiding this comment

guolinke commented Jul 13, 2020

imatiach-msft commented Jul 13, 2020

imatiach-msft left a comment

Choose a reason for hiding this comment

StrikerRUS commented Jul 13, 2020 • edited Loading

StrikerRUS Jul 13, 2020

Choose a reason for hiding this comment

guolinke Jul 14, 2020

Choose a reason for hiding this comment

StrikerRUS Jul 14, 2020

Choose a reason for hiding this comment

StrikerRUS commented Jul 13, 2020

guolinke commented Jul 14, 2020

StrikerRUS commented Jul 14, 2020

jameslamb left a comment

Choose a reason for hiding this comment

StrikerRUS commented Jul 15, 2020

github-actions bot commented Aug 24, 2023

StrikerRUS commented Jul 13, 2020 •

edited

Loading