[WIP] Extending AdaBoost's base estimator's feature_importances_ attr to support coef_ attr of an estimator #12326

dsprahul · 2018-10-08T13:05:28Z

Modified feature_importances_ attribute in AdaBoost's base estimator to work with coef_ attributes of given estimators if they don't provide feature_importances_

Reference Issues/PRs

Fixes #12137

What does this implement/fix? Explain your changes.

feature_importances_ attribute of BaseWeightBoosting class now converts coef_ attribute of input classifiers to their respective feature_importances_ using existing _get_feature_importances method in feature_selection/from_model.py.

This method accepts an optional argument: norm_order which defaults to 2 (L2 norm).

norm_order : {non-zero int, inf, -inf, "fro"}, optional
        Same as `numpy.linalg.norm`'s norm_order parameter

Any other comments?

This method is now renamed to get_feature_importances and moved to utils/__init__.py as it is a shared resource.

…r to accept "norm_type" parameter for converting `coef_` to `feature_importances`. Made a basic version of _coef_to_feature_importances function to handle this conversion

sklearn-lgtm · 2018-10-08T13:42:55Z

This pull request introduces 1 alert when merging e2d069b into 63e5ae6 - view on LGTM.com

new alerts:

1 for Unused local variable

Comment posted by LGTM.com

jnothman · 2018-10-09T02:10:32Z

sklearn/ensemble/weight_boosting.py

+            )
+        norm = non_negative_coefs_.sum()
+
+        return non_negative_coefs_ / norm


If the problem is multiclass, this return value will still, as far as I can tell, be 2-dimensional. feature_importances_ must be 1d. I had intended that the norm would be used to aggregate coefficients across classes, as is done in _get_feature_importances.

Alright, I will reuse _get_feature_importances function from feature_selection to extend feature_importances_ attribute of AdaBoost base class. Thanks for the link.

…` to support estimators with `coef_` parameter while calculating `feature_importances_` in AdaBoost based estimators. doc skip

sklearn-lgtm · 2018-10-09T08:13:44Z

This pull request introduces 1 alert when merging 61d9811 into 5fd9e03 - view on LGTM.com

new alerts:

1 for Encoding error

Comment posted by LGTM.com

jnothman

Thanks

jnothman · 2018-10-09T09:04:29Z

sklearn/ensemble/weight_boosting.py

@@ -33,6 +33,7 @@
 from ..externals.six.moves import zip
 from ..externals.six.moves import xrange as range
 from .forest import BaseForest
+from ..feature_selection.from_model import _get_feature_importances


If it is a shared utility, it should live in sklearn.utils

The implementation currently mentions SelectFromModel and should not

I see two solutions:

Move _get_feature_importances to sklearn.utils.__init__.py or create a new file (feature_importances.py) within utils and move it there; import this function into all those modules it's being used in (2 places)

Without reusing this function, duplicate its functionality with-in weight_boosting.py; if the above step is unnecessary.

@jnothman Which one should I choose?

jnothman · 2018-10-09T11:43:42Z

Put it in utils/init.py for now. May also be able to reuse in RFE (I think there may be an issue somewhere about it only supporting one norm?)

…o `get_feature_importances`

dsprahul · 2018-10-09T15:00:06Z

Alright, done.
Yes, I think it's the issue #2121. I will look into it/ pass-on the update once this one is MGR

jnothman · 2018-10-10T02:04:10Z

sklearn/utils/__init__.py

@@ -599,3 +599,39 @@ def is_scalar_nan(x):
    # Redondant np.floating is needed because numbers can't match np.float32
    # in python 2.
    return bool(isinstance(x, (numbers.Real, np.floating)) and np.isnan(x))
+
+
+def get_feature_importances(estimator, norm_order=1):


we still might want to name this _get_feature_importances, otherwise we are promising API stability to users.

Sure, will change it back to _get_feature_importances. Didn't think of that.
Also, should I write more tests to pass codecov? (Currently it's 84.21%)

jnothman

Code coverage would be nice too, yes.

Sorry for all this messing about. It would be great to have an API for parameterized feature importances (I made an issue about it at some point), but it is not a small thing to change. I'm undecided on what to advise here.

jnothman · 2018-10-10T09:59:26Z

sklearn/ensemble/weight_boosting.py

-    def feature_importances_(self):
-        """Return the feature importances (the higher, the more important the
-           feature).
+    def get_feature_importances_(self, norm_order=2):


I'm not sure that we should do this. We might need to make the norm a class parameter. Or just choose a norm and not give users an option.

It's okay. I will come-up with a solution by Monday.

@jnothman: How about we make a Mixin for feature_importances_ and inherit it where-ever it's required (Adaboost estimator, RFE, SelectFromModel etc..). This will require norm_order to be declared as a class variable in which-ever class it is being inherited into.

The job of this mixin is to determine whether a metaestimator or an usual estimator is inheriting it and then calculate feature_importances_ accordingly.

Something like this that can go into sklearn/base.py:

class FeatureImportancesMixin(object): """Mixin class that generates ``feature_importances_`` attribute.""" def _is_metaestimator(self): """Determines if `FeatureImportancesMixin` is inherited by an metaestimator. """ return hasattr(self, "estimators_") def _get_feature_importances(estimator, norm_order): """Retrieve or aggregate feature importances from estimator. Parameters ---------- estimator : any estimator object with either `feature_importances_` attribute or `coef_` attribute norm_order : {non-zero int, inf, -inf, "fro"}, optional Same as `numpy.linalg.norm`'s norm_order parameter Returns ------- importances : A 1d array, shape: [n_features] """ importances = getattr(estimator, "feature_importances_", None) coef_ = getattr(estimator, "coef_", None) if importances is None and coef_ is not None: if estimator.coef_.ndim == 1: importances = np.abs(coef_) else: importances = np.linalg.norm(coef_, axis=0, ord=self.norm_order) elif importances is None: raise AttributeError( "The underlying estimator %s has no `coef_` or " "`feature_importances_` attribute. Either pass a fitted estimator" " to SelectFromModel or call fit before calling transform." % estimator.__class__.__name__) return importances @property def feature_importances_(self): """Return the feature importances. The higher the value, the more important the feature. Returns ------- feature_importances_ : array, shape = [n_features] """ try: if self._is_metaestimator(): if self.estimators_ is None or len(self.estimators_) == 0: raise ValueError( "Estimator not fitted, " "call `fit` before `feature_importances_`.") norm = self.estimator_weights_.sum() estimator_feature_importances_ = [ _get_feature_importances(estimator, self.norm_order) for estimator in self.estimators_] return (sum(weight * feature_importances_ for weight, feature_importances_ in zip( self.estimator_weights_, estimator_feature_importances_)) / norm) else: return _get_feature_importances(estimator=self.estimator_, norm_order=self.norm_order) except AttributeError: raise AttributeError( "Unable to compute feature importances " "since base estimator neither has a " "feature_importances_ attribute nor a " "coef_ attribute")

jnothman · 2018-10-15T22:50:31Z

That design is not quite right, as I think you're confusing the meta estimator that uses the importances from the underlying estimator that provides only coef_.

Ultimately I think the norm is something users should be aware of when writing about how a model works, but is unlikely (?) something they would care to tweak.

This could be solved by deciding on one norm and sticking to it... Except that for a while we used different norms in SelectFromModel and RFE with no option to change. This is relatively hard to document... Although if we actually defined feature_importances_ on all estimators with coef_, that would be easy to document.

And if we went down the path of implementing get_importances() and deprecating feature_importances_, it would potentially involve adding an importances_params parameter to the meta estimators, but at least that would be clear and generic.

I think for now we should just choose a norm. Yes, we could make it configurable through a class-level attribute, and yes we should reuse that function from SelectFromModel, but I now think that over-engineering the meta-estimator is the wrong thing to do when the design issue is in the constituent estimator.

Wrote a test case to test the feature_importances_ calculation using coef_ attr of underlying estimator

dsprahul · 2018-10-16T13:11:16Z

Alright, I made L2 norm as the default for AdaBoosts which could be configured via class-variable norm_order. I also added a test to test coef_ to feature_importances_ conversion.

hermidalc · 2018-10-30T12:46:28Z

Actually @jnothman @RahulDamineni @alexsieusahai I don't think we should provide a parameter for the norm to make coef_ values non-negative, RFE and RFECV do not. We should use the exact same code as what is there, at least as a first implementation:

scikit-learn/sklearn/feature_selection/rfe.py

Lines 181 to 195 in bac89c2

    
           # Get coefs 
        
           if hasattr(estimator, 'coef_'): 
        
               coefs = estimator.coef_ 
        
           else: 
        
               coefs = getattr(estimator, 'feature_importances_', None) 
        
           if coefs is None: 
        
               raise RuntimeError('The classifier does not expose ' 
        
                                  '"coef_" or "feature_importances_" ' 
        
                                  'attributes') 
        
           # Get ranks 
        
           if coefs.ndim > 1: 
        
               ranks = np.argsort(safe_sqr(coefs).sum(axis=0)) 
        
           else: 
        
               ranks = np.argsort(safe_sqr(coefs))

dsprahul added 2 commits October 8, 2018 18:21

Redesigned feature_importances_ attribute in AdaBoost's base estimato…

d2cca96

…r to accept "norm_type" parameter for converting `coef_` to `feature_importances`. Made a basic version of _coef_to_feature_importances function to handle this conversion

missed comma

e2d069b

dsprahul added 2 commits October 8, 2018 23:01

Missed renaming variables bug fix. doc skip

1097899

Shortened a variable name to comply with <80c/l limit. doc skip

52b32f6

jnothman reviewed Oct 9, 2018

View reviewed changes

Used _get_feature_importances from `feature_selection/from_model.py…

61d9811

…` to support estimators with `coef_` parameter while calculating `feature_importances_` in AdaBoost based estimators. doc skip

Fixes encoding issue

f1d5049

jnothman reviewed Oct 9, 2018

View reviewed changes

Moved _get_feature_importances to utils/__init__.py and renamed t…

e07d925

…o `get_feature_importances`

jnothman reviewed Oct 10, 2018

View reviewed changes

renamed get_feature_importances to it's original

5b91d5e

jnothman reviewed Oct 10, 2018

View reviewed changes

jnothman mentioned this pull request Oct 10, 2018

feature_importances_ should be a method in the ideal design #9606

Open

Made norm_order a class variable.

712efca

Wrote a test case to test the feature_importances_ calculation using coef_ attr of underlying estimator

amueller added the Waiting for Reviewer label Aug 5, 2019

github-actions bot added module:ensemble module:feature_selection module:utils labels Mar 2, 2020

Base automatically changed from master to main January 22, 2021 10:50

cmarmo removed the Waiting for Reviewer label Mar 1, 2021

tusharparimi mentioned this pull request Sep 12, 2024

AdaBoost feature_importances_ should support base estimators with coef_ #12137

Open

Uh oh!

[WIP] Extending AdaBoost's base estimator's feature_importances_ attr to support coef_ attr of an estimator #12326

Are you sure you want to change the base?

[WIP] Extending AdaBoost's base estimator's feature_importances_ attr to support coef_ attr of an estimator #12326

Uh oh!

Conversation

dsprahul commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

sklearn-lgtm commented Oct 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklearn-lgtm commented Oct 9, 2018

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsprahul Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Oct 9, 2018

Uh oh!

dsprahul commented Oct 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Oct 15, 2018

Uh oh!

dsprahul commented Oct 16, 2018

Uh oh!

hermidalc commented Oct 30, 2018

Uh oh!

Uh oh!

dsprahul commented Oct 8, 2018 •

edited

Loading

dsprahul Oct 9, 2018 •

edited

Loading