8000 ⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev ⚠️ · Issue #24424 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev ⚠️ #24424

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.

Sign up for GitHub

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

Jump to bottom
Closed
scikit-learn-bot opened this issue Sep 12, 2022 · 6 comments · Fixed by #24543
Closed

⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev ⚠️ #24424

scikit-learn-bot opened this issue Sep 12, 2022 · 6 comments · Fixed by #24543
Labels
Bug

Comments

@scikit-learn-bot
Copy link
Contributor
scikit-learn-bot commented Sep 12, 2022 •
edited
Loading

CI is still failing on Linux_Nightly.pylatest_pip_scipy_dev (Oct 13, 2022)

  • test_searchcv_raise_warning_with_non_finite_score[GridSearchCV-specialized_params0-True]
  • test_searchcv_raise_warning_with_non_finite_score[RandomizedSearchCV-specialized_params1-True]
@github-actions github-actions bot added the Needs Triage Issue requires triage label Sep 12, 2022
@lesteve
Copy link
Member
lesteve commented Sep 12, 2022

One of the issue is a DeprecationWarning because scipy.misc.face has moved to scipy.datasets.face
https://scipy.github.io/devdocs/reference/generated/scipy.misc.face.html
image

I guess we need a utils.fixes for this. There may be other issues, not 100% sure.

@lesteve lesteve added Bug and removed Needs Triage Issue requires triage labels Sep 12, 2022
@ogrisel ogrisel mentioned this issue Sep 15, 2022
Closed
@cmarmo cmarmo mentioned this issue Sep 15, 2022
Merged
@cmarmo
Copy link
Contributor
cmarmo commented Sep 16, 2022

@lesteve for test_grid_search_failing_classifier could the reason be scipy/scipy#16406 merged after your fix in #24141?

@lesteve
Copy link
Member
lesteve commented Sep 16, 2022 •
edited
Loading

Yep, it seems like we need to replace NaNs by np.inf before calling scipy.stats.rankdata.

In scipy 1.10.dev, now scipy.stats.rankdata returns only NaNs as soon as there is a single NaN:

In [1]: import numpy as np
   ...: 
   ...: from scipy.stats import rankdata
   ...: 
   ...: print(rankdata([1, 2, np.nan]))
[nan nan nan]

@ogrisel
Copy link
Member
ogrisel commented Sep 16, 2022

Why not just call nan_policy="omit" explicitly instead of the new default nan_policy="propagate"?

To support older versions of scipy without the nan_policy argument, we could have sklearn.util.fixes that implements the previous behavior.

@lesteve
Copy link
Member
lesteve commented Sep 16, 2022

It feels like the simplest option is a local fix but 🤷.

I quickly looked and I think SearchCV objects is the only place in scikit-learn where scipy.stats.rankdata is called with NaNs. The only other place that rankdata is used is sklearn.metrics.label_ranking_average_precision_score and this uses check_array so there won't be any NaNs there.

Note that scipy 1.9 does something different than scipy 1.10dev with either nan_policy="omit" or nan_policy="propagate", so we need some special logic to get the previous behaviour (NaNs get maximum rank).

@cmarmo cmarmo mentioned this issue Sep 20, 2022
Merged
This was referenced Sep 27, 2022
Merged
Merged
@thomasjpfan thomasjpfan mentioned this issue Oct 12, 2022
Merged
@thomasjpfan
Copy link
Member

For linking purposes: #24543 should fix the remaining issue with rankdata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

MAINT use nanmin to replace nan by finite values in ranking of SearchCV glemaitre/scikit-learn
5 participants
@ogrisel @cmarmo @lesteve @thomasjpfan @scikit-learn-bot
0