Remove BayesSearchCV(iid=) parameter deprecated in sklearn 0.24

[kernc]

Fixes #978
Fixes #987
Fixes #927
Fixes #990
Fixes #981 via updated scikit-learn

Fixes #1006

Fixes #718

Closes #951

[betatim]

LGTM except for that one language suggestion.

[andreimargeloiu]

Please merge this!

[bole1]

my commit on this merge request. Sorry, but Ive got 2 troubles with this merge request on sklearn 0.24

[thomasfrederikhoeck]

@trinebrockhoff

[kernc]

@betatim We need to ship this fix. Afterwards, we need someone to tackle #718.

[JonasTriki]

Any update on when this PR will get merged? @betatim

[kernc]

I suppose not as long as it's failing the CI. I'm tackling related years-overdue #718. Give me a few more days. 😅

[kernc]

Looks promising!

With the latest commit, I've migrated the class to use sklearn's BaseSearchCV._run_search() API (#718). Thanks to @homarb's sketch, not a lot was needed. Unfortunate it didn't find its way in already in 2018.

[glouppe]

Feel free to merge this when this looks OK and then move on with all the other patches :-) Thanks for the work @kernc !

[kernc]

@glouppe Would the notable change go into the 0.8.2 or the 0.9.0 whatsnew, in your opinion?

[glouppe]

Time for 0.9!

[glouppe]

Shall we merge? :)

[kernc]

I believe so. I think also a prompt release would be well accepted. This PR closes 7 issues with tens of subscribers. 😬

git log v0.8.1..HEAD shows a few more notes to add to 0.9.0 changelog.

[hsorsky]

Is there anything blocking this? It appears to be passing CI with the author (@kernc) and a skopt contributor (@glouppe) agreeing that its ready to merge

[QuentinSoubeyran]

I suggest adding support for multimetric scoring to BayesSearchCV to provide an interface closer to that of GridSearchCV and RandomSearchCV. Currently, trying multimetric scoring (e.g. because GridSearchCV was replaced by BayesSearchCV) yields an obscure KeyError at L411, due to the result dict naming scheme changing between multimetric and single metric.

This doesn't appear too hard, as only L411 and L471 need to be adapted. I believe this is sufficient:

• adding a new parameter to BayesSearchCV, for instance optimized_scoring, that defaults to "score". If multimetric scoring is used, this parameter must be set to the key of the scorer to use, similar to the refit argument (see below why the refit argument is not used)
• adding a check for the above requirement to specify the additional argument. It is possible to use None as the default value, check for multimetric scoring by using the same code as BaseSearchCV.fit(), and defaulting or raising if no value was passed. Or we simply check for "score", assuming no metric is named "score" in a multimetric setting.
• changing L411 to local_results = all_results["mean_test_" + self.optimized_scoring][-len(params):] or local_results = all_results["mean_test_" + (self.optimized_scoring or "score")][-len(params):] if a None default is used
• adapting L471 to handle the multimetric case. This check is easier than the one for the new parameter, because at that point self.multimetric_ -- provided by BaseSearchCV -- is available

Using refit for this purpose requires handling callable refit (at least to raise an error if unsupported) and might change the effect of that parameter compared to BayesSearchCV and RandomSearchCV, so I think having a dedicated parameter is easier to implement and provide more functionality.

If @kernc and @glouppe approve this idea, I am happy to make a small PR.

[kernc]

• adding a new parameter to BayesSearchCV, for instance optimized_scoring

The multimetric feature is more thoroughly discussed in #797. I'm leaning to reusing refit= as far as possible. If you can make a PR that effectively fixes #797, that'd be welcome.

[QuentinSoubeyran]

Sorry for missing #797 !
I think I have a working PR, using refit as indicated. I tried to keep as much of GridSearchCV's functionalities as I could, and to provide clear errors for the few bits that are not supported (callable refit for instance).
Since the changes depend on your PR, should I PR on your fork @kernc , or should I make a separate PR in this repo ?

[kernc]

PR on your fork

I think that would be the easiest wrt integration.

[glouppe]

@kernc Feel free to click the green button and merge this PR. The work comes mainly from you :) (It will then be easier for everyone else to build upon...)

[kernc]

@QuentinSoubeyran You should now make a fix against master directly. 👍

[wundermahn]

Is this merged into master? I just installed today, still a problem :(

[glouppe]

Yes, this has been merged. Can you report the issue you have?

[grudloff]

Hello, is there a schedule for a stable release including this fix? Currently, release 0.8.1 is still broken.

[wundermahn]

@glouppe This is what I was referring to. When I use 0.8.1 and still get this problem with sklearn version 0.24.2

[donmegamuffin]

Same here, still an issue with sklearn 0.24.1 and skopt 0.8.1. Line 432 of searchcv.py throws a wobbly unpacking the zip(*out)

[wundermahn]

Can anyone post an example? I'm away from home at the moment.

[indrajitsg]

Even I am facing this issue.

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Miniconda3\envs\pyga\lib\site-packages\skopt\searchcv.py", line 692, in fit
    optim_result = self._step(
  File "C:\Miniconda3\envs\pyga\lib\site-packages\skopt\searchcv.py", line 579, in _step
    self._fit(X, y, groups, params_dict)
  File "C:\Miniconda3\envs\pyga\lib\site-packages\skopt\searchcv.py", line 431, in _fit
    (test_scores, test_sample_counts,
ValueError: too many values to unpack (expected 5)

[serhii-datajob]

Hi!
Are there any plans to fix the issue with unpacking the zip(*out)?
Thanks!

[QuentinSoubeyran]

While this PR was merged into the master branch, scikit-optimize 0.9 has not been released yet. The scikit-optimize version on PyPi (0.8.1 at the time of writing) is still incompatible with scikit-learn 0.24+.

For anyone needing the compatibility, you can install scikit-optimize directly from source with

pip install git+https://github.com/scikit-optimize/scikit-optimize.git

If you also need multimetric BayesSearch, you need to install from my fork until #1030 is merged:

pip install git+https://github.com/QuentinSoubeyran/scikit-optimize.git@multimetric

Hope this helps.

[glouppe]

@kernc Could you make a release? I think that would help a lot of users :-)

[glouppe]

(And in general, we need help from all of you with the project! No one is actively maintaining it anymore, unfortunately.)

[wundermahn]

Can you clarify that nobody is maintaining anymore? I see that a number of employers and institutions (including big companies like Northrop Grumman) have worked on it in the past?

…

On Thu, Jul 29, 2021 at 12:20 PM Gilles Louppe ***@***.***> wrote: (And in general, we need help from all of you with the project! No one is actively maintaining it anymore, unfortunately.) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#988 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGBJO6DMVCGGH77XUMY7VNTT2F5V3ANCNFSM4WOLPEVA> .

-- Tony Kelly 829 Concorde Circle Unit 4202 Linthicum Heights, MD Cell: 443 717 1215 ***@***.*** *We can only see a short distance ahead, but we can see plenty there that needs to be done. - Alan Turing*

[serhii-datajob]

For anyone needing the compatibility, you can install scikit-optimize directly from source with

@QuentinSoubeyran , thanks a lot!
Your version from source is not rising the deprecated iid or zip unpacking errors anymore!

[gwerbin]

(And in general, we need help from all of you with the project! No one is actively maintaining it anymore, unfortunately.)

@glouppe is the problem fixed? Do you just need someone to publish a v0.9 release to PyPI? Or does more work need to happen still? I might be willing to answer a "call for maintainers", but it'd be helpful to know what's involved.

I'm also very interested in @QuentinSoubeyran's multimetric version, and if/how that can be merged upstream.

[jhn-nt]

Any news on when 9.0 with sklearn >0.24 support will be released?

[QuentinSoubeyran]

@wundermahn My understanding is that no one is regularly taking the time to answer and fix issues, review and merge PR and make releases on PyPI

[RNarayan73]

Hello
Is the multi-metric feature included in the latest release 0.9?
It doesn't appear in the change log (https://scikit-optimize.github.io/dev/whats_new/v0.9.html)
But is in a 0.9rc1 here (#1071)
Thanks
Narayan

[QuentinSoubeyran]

@RNarayan73
The multimetric feature is not part of 0.9.0, #1062 hasnt been merged in master yet.

The 0.9.0rc1 you mention is completely unofficial: it is a branch I made on my fork to test things out. Since the release of the official 0.9.0 by kernc, I haven't rebase my fork on the current master branch, so it is out-of-sync with scikit-optimize.
#1071 was just a proposal, and should probably be closed with the release of 0.9.0.

[RNarayan73]

Ok. Thanks for clarifying.
I notice that your branch mock0.9rc1 has the multimetric that I can install.
Narayan