Skip to content

No attribute classes_ during multi-class scoring #26336

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
larsoner opened this issue May 5, 2023 · 8 comments
Closed

No attribute classes_ during multi-class scoring #26336

larsoner opened this issue May 5, 2023 · 8 comments
Labels
Bug Needs Triage Issue requires triage

Comments

@larsoner
Copy link
Contributor

larsoner commented May 5, 2023

Describe the bug

Regression we hit in MNE-Python's pip-pre run and bisected locally:

https://github.com/mne-tools/mne-python/actions/runs/4894775241/jobs/8739516519#step:17:4138

Local bisect suggests the culprit is #26037 by @glemaitre

It's entirely possible we're doing something wrong at the MNE-Python end but this code has worked for years 🤷

cc @agramfort who might also know the problem quickly

Steps/Code to Reproduce

Not really minimal, but after cloning MNE-Python you can do:

pytest mne/decoding/tests/test_base.py -k multiclass_full

And it will fail.

Can try to whittle it down more if it would help...

Expected Results

No error

Actual Results

mne/decoding/tests/test_base.py:323: in test_get_coef_multiclass_full
    scores = cross_val_multiscore(time_gen, X, y, cv=cv, verbose=True)
...
mne/decoding/base.py:567: in _score
    score = scorer(estimator, X_test, y_test)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/metrics/_scorer.py:411: in _passthrough_scorer
    return estimator.score(*args, **kwargs)
mne/decoding/search_light.py:581: in score
    score = parallel(
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/parallel.py:1085: in __call__
    if self.dispatch_one_batch(iterator):
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/parallel.py:901: in dispatch_one_batch
    self._dispatch(tasks)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/parallel.py:819: in _dispatch
    job = self._backend.apply_async(batch, callback=cb)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/_parallel_backends.py:208: in apply_async
    result = ImmediateResult(func)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/_parallel_backends.py:597: in __init__
    self.results = batch()
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/parallel.py:288: in __call__
    return [func(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/joblib/parallel.py:288: in <listcomp>
    return [func(*args, **kwargs)
mne/decoding/search_light.py:676: in _gl_score
    _score = scoring(est, X[..., jj], y)
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/metrics/_scorer.py:211: in __call__
    return self._score(
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/metrics/_scorer.py:291: in _score
    y_pred = method_caller(clf, "predict_proba", X, pos_label=self._get_pos_label())
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/metrics/_scorer.py:77: in _cached_call
    result, _ = _get_response_values(
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/utils/_response.py:74: in _get_response_values
    classes = estimator.classes_
/opt/hostedtoolcache/Python/3.11.3/x64/lib/python3.11/site-packages/sklearn/pipeline.py:744: in classes_
    return self.steps[-1][1].classes_
E   AttributeError: 'LinearModel' object has no attribute 'classes_'

Versions

Dev
@larsoner larsoner added Bug Needs Triage Issue requires triage labels May 5, 2023
@glemaitre
Copy link
Member

Thanks for reporting.
A contract in scikit-learn (enforce by check_estimator) is that a classifier must have a classes_ that is a NumPy array.

Could you provide what is the class of estimator here such that we see if we don't take right branch or if the estimator is in some way not scikit-learn compatible?

@glemaitre
Copy link
Member

I had a look at the LinearModel class: https://github.com/mne-tools/mne-python/blob/16fe5b5dbb1622ddfb7269fab889bab44ef43212/mne/decoding/base.py#L17

I think that it misses a method to wire the classes_ exposed in model:

@property
def classes_(self):
    return self.model.classes_

@glemaitre
Copy link
Member

For completeness, the current implementation of LinearModel would fail the following common tests:

assert hasattr(classifier, "classes_")

@larsoner
Copy link
Contributor Author

larsoner commented May 5, 2023

That does indeed fix the issue, I'll add that!

Not sure if there is anything to fix at the sklearn end. We do inherit from BaseEstimator (and though we vendor it, using from sklearn.base import BaseEstimator didn't fix the problem) but I don't know there is anything for you to add to it. Feel free to close if you agree.

@larsoner
Copy link
Contributor Author

larsoner commented May 5, 2023

... but at our end do you suggest we use this estimator_checks function to make sure our estimators are compliant?

@glemaitre
Copy link
Member

Yep BaseEstimator is not enough here. This is more a protocol-like thing: https://scikit-learn.org/stable/developers/develop.html#specific-models that is not inherited.

but at our end do you suggest we use this estimator_checks function to make sure our estimators are compliant?

It makes sense to try it. If the tests are passing then, it is quite safe to use will any component of scikit-learn. Note that we always have a mechanism to skip some of the common tests if it fails for a good reason.

@larsoner
Copy link
Contributor Author

larsoner commented May 5, 2023

Thanks for the quick response, and sorry that we should have caught that it was our faulty implementation in the end!

@larsoner larsoner closed this as completed May 5, 2023
@glemaitre
Copy link
Member

Happy to be helpful ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Needs Triage Issue requires triage
Projects
None yet
Development

No branches or pull requests

2 participants