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

[MRG + 1] Add refit_time_ attribute to model selection #11310

Merged
merged 6 commits into from Jun 20, 2018

Conversation

Projects
None yet
3 participants
@mfeurer
Contributor

mfeurer commented Jun 18, 2018

Reference Issues/PRs

Resolves #8833.

What does this implement/fix? Explain your changes.

This PR implements a new refit_time_ attribute which will be stored in GridSearchCV and RandomizedSearchCV if refit is set to True. This will allow measuring the complete time it takes to perform + hyperparameter optimization and refitting the best model on the whole dataset and will be particularly helpful if the hyperparameter optimization is carried out in parallel.

Add refit_time to model selection
In particular to GridSearchCV and RandomizedSearchCV. Implements #8833.
@jnothman

Why WIP? What's the todo list?

refit_time_ : float
Seconds used for refitting the best model on the whole dataset.
This is present only if ``refit`` is set to ``True``.

This comment has been minimized.

@jnothman

jnothman Jun 18, 2018

Member

I think you mean "if ``refit`` is not False"

This comment has been minimized.

@mfeurer

mfeurer Jun 18, 2018

Contributor

Thanks a lot, I just fixed the wording.

@mfeurer

This comment has been minimized.

Contributor

mfeurer commented Jun 18, 2018

I wasn't sure if the tests/CI will pass, therefore I decided that this PR isn't ready yet. If that's wrong I'm happy to change the name prior to the tests passing.

@mfeurer

This comment has been minimized.

Contributor

mfeurer commented Jun 18, 2018

There appears to be an issue with time measurement and Windows. Can I disable the check that the refit time is larger than zero for Windows?

:class:`model_selection.RandomizedSearchCV` if ``refit`` is set to ``True``.
This will allow measuring the complete time it takes to perform
hyperparameter optimization and refitting the best model on the whole
dataset.

This comment has been minimized.

@amueller

amueller Jun 18, 2018

Member

add :user: thing and PR # (since there's no issue).

assert_true(hasattr(search, "refit_time_"))
assert_true(isinstance(search.refit_time_, float))
if sys.version_info[0] >= 3:
assert_greater(search.refit_time_, 0)

This comment has been minimized.

@amueller

amueller Jun 18, 2018

Member

why not >= as for the other parameters?

This comment has been minimized.

@mfeurer

mfeurer Jun 19, 2018

Contributor

Just did this - why didn't I realize this before... Please excuse the extra commits.

mfeurer added some commits Jun 19, 2018

@mfeurer

This comment has been minimized.

Contributor

mfeurer commented Jun 19, 2018

Unit tests are passing except for one test on travis-ci which fails because of a server error on mldata.org.

@mfeurer mfeurer changed the title from [WIP] Add refit_time_ attribute to model selection to Add refit_time_ attribute to model selection Jun 19, 2018

@mfeurer mfeurer changed the title from Add refit_time_ attribute to model selection to [MRG] Add refit_time_ attribute to model selection Jun 19, 2018

@amueller

This comment has been minimized.

Member

amueller commented Jun 19, 2018

ugh that's annoying. I restarted that one.

@amueller

lgtm

@amueller amueller changed the title from [MRG] Add refit_time_ attribute to model selection to [MRG + 1] Add refit_time_ attribute to model selection Jun 19, 2018

@jnothman

LGTM, thanks!

@jnothman jnothman merged commit 768ff4d into scikit-learn:master Jun 20, 2018

5 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
LGTM analysis: Python No alert changes
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: python2 Your tests passed on CircleCI!
Details
ci/circleci: python3 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

georgipeev pushed a commit to georgipeev/scikit-learn that referenced this pull request Jun 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment