-
-
Notifications
You must be signed in to change notification settings - Fork 261
ENH: provide monitoring of model selection searches #528
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
Conversation
parameters are not recognized during fitting
parameters are not recognized during fitting
I'm not sure this PR has the best method of printing/logging information: dask-ml/dask_ml/model_selection/_incremental.py Lines 112 to 115 in bde46b7
I'm also unsure how to test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be able to write a test for the KeyboardInterrupt using signal.sigint
. LMK if you want help with that.
I'm having difficult time with implementation that catches the KeyboardInterrupt Traceback (most recent call last)
<ipython-input-5-3748455ece57> in <module>()
----> 9 df = _test_keyboardinterrupt()
<ipython-input-4-135866acf811> in _test_keyboardinterrupt()
---> 13 search.fit(X, y)
~/Developer/stsievert/dask-ml/dask_ml/model_selection/_incremental.py in fit(self, X, y, **fit_params)
--> 641 return default_client().sync(self._fit, X, y, **fit_params)
~/anaconda3/lib/python3.6/site-packages/distributed/client.py in sync(self, func, *args, **kwargs)
--> 753 return sync(self.loop, func, *args, **kwargs)
~/anaconda3/lib/python3.6/site-packages/distributed/utils.py in sync(loop, func, *args, **kwargs)
--> 329 e.wait(10)
~/anaconda3/lib/python3.6/threading.py in wait(self, timeout)
--> 551 signaled = self._cond.wait(timeout)
~/anaconda3/lib/python3.6/threading.py in wait(self, timeout)
--> 299 gotit = waiter.acquire(True, timeout)
KeyboardInterrupt: |
I'm learning a bit about this for a distributed issue. Will take a look later today. |
Can you talk a bit about what the expected behavior of a KeyboardInterrupt halfway through is? As a user, I might expect the following
Overall, this looks a bit more complicated that adding better logging. It may be worth splitting into its own PR. |
I would certainly expect 1 and 2. I intend the |
Looks like a merge conflict. |
|
The CI is failing because |
Ah, I didn't realize it was that new. I think defining something like this in @contextlib.contextmanager
def nullcontext():
yield should suffice |
I've also deprecated the |
): | ||
if scores_per_fit is not None and fits_per_score != 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, didn't realize this deprecation was in the __init__
. This should be done in the fit
. __init__
isn't supposed to do any validation.
This will also fix the issue you saw with the warnings in the doc build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'oh, of course. I've made that change, and removed the :okwarning:
s. This PR should be ready merge now.
Thanks! |
What does this PR implement?
It allows the user to monitor the model selection search. It prints out information on the score received
and allows the user to stop the searches via Cntrl-C.verbose
is False (default): log all the time but don't pipe to stdout (aka print)verbose
is True: log and print all the timeverbose
is an int/float and0 < verbose <= 1
: log and printverbose
fraction of the time.verbose
is zero: do not log past initialization.This allows:
verbose=True
verbose=False
.verbose=0
.All types are cast to float except bool; valid values include
verbose in [0.0, 1, True, False, 0.5]
. An error will be raised ifnot 0 <= verbose <= 1
.