Skip to content

SelectFromModel does not work when ElasticNetCV has multiple l1 ratios #30936

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

Closed
muhlbach opened this issue Mar 4, 2025 · 2 comments · Fixed by #31107
Closed

SelectFromModel does not work when ElasticNetCV has multiple l1 ratios #30936

muhlbach opened this issue Mar 4, 2025 · 2 comments · Fixed by #31107
Labels

Comments

@muhlbach
Copy link

muhlbach commented Mar 4, 2025

Describe the bug

Using SelectFromModel with the automatic ElasticNetCV does not work if the l1_ratio is estimated from the data, i.e., if the user provides a list of floats.

Steps/Code to Reproduce

from sklearn.datasets import make_regression
from sklearn.feature_selection import SelectFromModel
from sklearn.linear_model import ElasticNetCV
estimator = ElasticNetCV(
    l1_ratio=[0.25, 0.5, 0.75]
)
model = SelectFromModel(estimator=estimator)
X, y = make_regression(n_samples=100, n_features=5, n_informative=3)
model.fit(X, y)
model.get_feature_names_out()

This fails with:

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

because _calculate_threshold calls np.isclose(estimator.l1_ratio, 1.0) which returns an array with as many elements as l1 ratios.

Expected Results

Calling .get_feature_names_out() should return an ndarray of str according to the best model estimating with CV.

Actual Results

Traceback (most recent call last):
  File "/.venv/lib/python3.12/site-packages/IPython/core/interactiveshell.py", line 3577, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-12-304146ab06de>", line 1, in <module>
    model.get_feature_names_out()
  File "/.venv/lib/python3.12/site-packages/sklearn/feature_selection/_base.py", line 190, in get_feature_names_out
    return input_features[self.get_support()]
                          ^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.12/site-packages/sklearn/feature_selection/_base.py", line 67, in get_support
    mask = self._get_support_mask()
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.12/site-packages/sklearn/feature_selection/_from_model.py", line 305, in _get_support_mask
    threshold = _calculate_threshold(estimator, scores, self.threshold)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/.venv/lib/python3.12/site-packages/sklearn/feature_selection/_from_model.py", line 36, in _calculate_threshold
    if is_l1_penalized or is_lasso or is_elasticnet_l1_penalized:
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

Versions

System:
    python: 3.12.9 (main, Feb  4 2025, 14:38:38) [Clang 16.0.0 (clang-1600.0.26.6)]
executable: /.venv/bin/python
   machine: macOS-15.1.1-arm64-arm-64bit
Python dependencies:
      sklearn: 1.5.2
          pip: None
   setuptools: 75.6.0
        numpy: 1.26.4
        scipy: 1.14.1
       Cython: None
       pandas: 2.2.3
   matplotlib: 3.9.3
       joblib: 1.4.2
threadpoolctl: 3.5.0
Built with OpenMP: True
threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /.venv/lib/python3.12/site-packages/numpy/.dylibs/libopenblas64_.0.dylib
        version: 0.3.23.dev
threading_layer: pthreads
   architecture: armv8
       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libomp
       filepath: /.venv/lib/python3.12/site-packages/sklearn/.dylibs/libomp.dylib
        version: None
@muhlbach muhlbach added Bug Needs Triage Issue requires triage labels Mar 4, 2025
@vasco-s-pereira
Copy link
Contributor

Hi,
I’m a first-time contributor to this repository and would love to work on this issue if it’s still available. Could you please let me know if I can take it on?

@lesteve
Copy link
Member

lesteve commented Mar 13, 2025

Thanks for the issue, this looks like a real bug. I have to say I am not sure how to handle this case off the top of my head ...

@vasco-s-pereira great if you are looking to contribute to scikit-learn, I would suggest you look at the Contributing doc and New Issues for contributors.

@lesteve lesteve removed the Needs Triage Issue requires triage label Mar 13, 2025
vasco-s-pereira added a commit to vasco-s-pereira/scikit-learn that referenced this issue Mar 26, 2025
…array with only floats.

    Added the _is_l1_ratio_one function in _from_model.py where it checks if the l1_ratio is an array, list or tuple and, in that case, extends np.isclose to the whole array using np.any so that, if atleast on value is approximately equal to 1.0 it returns True.
vasco-s-pereira added a commit to vasco-s-pereira/scikit-learn that referenced this issue Mar 26, 2025
…rray with only floats.

Added the _is_l1_ratio_one function in _from_model.py and a few tests.
vasco-s-pereira added a commit to vasco-s-pereira/scikit-learn that referenced this issue Mar 26, 2025
Added `_is_l1_ratio_one` function to handle l1_ratio as array, list, or
tuple. Uses `np.isclose` and `np.any` to check if at least one element is
approximately equal to 1.0.
vasco-s-pereira added a commit to vasco-s-pereira/scikit-learn that referenced this issue Mar 26, 2025
Added `_is_l1_ratio_one` function to handle l1_ratio as array, list, or
tuple. Uses `np.isclose` and `np.any` to check if at least one element
is approximately equal to 1.0.
vasco-s-pereira added a commit to vasco-s-pereira/scikit-learn that referenced this issue Apr 21, 2025
…that ElasticNetCV works with a list of floats; Add non‑regression test `test_get_feature_names_out_elasticnetcv`
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 a pull request may close this issue.

3 participants