[ENH] Neural network widget that works in a separate thread#2958
[ENH] Neural network widget that works in a separate thread#2958lanzagar merged 5 commits intobiolab:masterfrom
Conversation
The scikit-learn class in the background does not support callbacks, so I made its verbose output the callback.
|
@ales-erjavec Could you check this PR and comment, please? Please, also see the changes to the widget's cancel(). There I needed to process events so that the progress bars updates worked OK when I used the cancel button. |
| if learner.kwargs["solver"] != "lbfgs": | ||
| # enable verbose printouts within scikit and redirect them | ||
| with patch.dict(learner.kwargs, {"verbose": True}),\ | ||
| patch("builtins.print", print_callback): |
There was a problem hiding this comment.
Time -->
Thread-1
-|patch|-----|exit|--------
Thread-2
----|patch|--------|exit|--
^ ^
stores restores
patched print the patched print of Thread-1
There was a problem hiding this comment.
Whoops. Thank you. Did not know that patch mutates state globally....
|
The alternative to your last commit would be to set |
|
@kernc Yes, that would certainly be an option. But then I find automatic stopping due to too tiny updates problematic: we would have to detect it and avoid running partial_fit again. |
|
What about learning rate schedule? From what I can tell the 'adam' solver uses learning rate decay, so any model trained in such manner could be very different then one trained in a single run. |
MLPClassifier n_iters_ was made a property which calls a callback.
Codecov Report
@@ Coverage Diff @@
## master #2958 +/- ##
==========================================
+ Coverage 81.86% 81.88% +0.02%
==========================================
Files 329 329
Lines 56785 56905 +120
==========================================
+ Hits 46486 46597 +111
- Misses 10299 10308 +9 |
|
@ales-erjavec, what do you think about this attempt? |
ales-erjavec
left a comment
ales-erjavec
left a comment
There was a problem hiding this comment.
I do not particularly like the idea of raising exceptions from n_iter_ setter.
|
|
||
| def callback(iteration): | ||
| if task.cancelled: | ||
| raise CancelThreadException() # this stop the thread |
There was a problem hiding this comment.
The comment (and exception name) are not correct. This does not stop the thread. It cancels/interrupts the task.
| self._task = None | ||
| # threads use signals to run functions in the main thread and some | ||
| # can still be quoued (perhaps change) | ||
| qApp.processEvents() |
There was a problem hiding this comment.
Remove this. If needed route the progress updates via an intermediary QObject maybe like this
There was a problem hiding this comment.
Thanks! I haven't thought of this.
|
I do not like faking n_iter_ either, but haven't thought of anything better here... Feel free to decide that this is too dirty to get into Orange. |
| return learner(data) | ||
| try: | ||
| return learner(data) | ||
| except CancelTaskException: |
There was a problem hiding this comment.
What is the difference between former (no try-except) and this?
There was a problem hiding this comment.
The later does not capture the exception instance's __traceback__ in the resulting Future (which we know will be discarded).
astaric
changed the title
6 participants
Issue
In general, long computation shouldn't leave the GUI unresponsive. But the real issue is that I am not sure I can write threaded widgets, so I tried.
Description of changes
Implemented with the help of @ales-erjavec's example from the documentation with minor changes. Callbacks with scikit-learn are very hackish though. Does anybody have a better idea?
Just to test stopping, I also added a cancel button to the widget. Perhaps all interruptible widget should get a stop icon in the status bar and perhaps also some actionable "icon" on the canvas.
Includes