Interrupt optimization through IncorporateRunResultCallback #765
Conversation
Corrected Version number
…orporateRunResultCallback
Codecov Report
@@ Coverage Diff @@
## development #765 +/- ##
===============================================
+ Coverage 86.97% 87.02% +0.05%
===============================================
Files 68 68
Lines 6218 6222 +4
===============================================
+ Hits 5408 5415 +7
+ Misses 810 807 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hi @thomashlvt,
Glad you could get this done so quickly. I looked through the code and the test and it seems fairly straightforward and non-disruptive which is very nice! I have added one question in the review.
I imagine this feature will wanted to be documented somewhere but there should be someone more involved with SMAC to review this properly.
If there is no activity on this soon I will bump someone to take a look
| @@ -50,7 +50,8 @@ class TestSMBO(unittest.TestCase): | |||
| def setUp(self): | |||
| self.scenario = Scenario({'cs': test_helpers.get_branin_config_space(), | |||
| 'run_obj': 'quality', | |||
| 'output_dir': 'data-test_smbo'}) | |||
| 'output_dir': 'data-test_smbo', | |||
| "runcount-limit": 5}) | |||
| self.output_dirs = [] | |||
There was a problem hiding this comment.
Was there a need to include a "runcount-limit"?
There was a problem hiding this comment.
If the TestCallback would not return False at some point, or if stopping the optimization loop would not work, the test would run forever. This is not the case, but I thought it'd be better to have this safety net. I'm not sure if this has an impact on the other tests. Let me know if you would like me to remove this, or constrain it to test_incorporate_run_results_callback_stop_loop.
There was a problem hiding this comment.
This makes sense to me but hopefully someone more familiar with all the tests can decide.
| @@ -1,4 +1,5 @@ | |||
| from typing import TYPE_CHECKING | |||
| from typing import Optional | |||
|
|
|||
There was a problem hiding this comment.
The two instances of from typing import X can be reduced to 1
There was a problem hiding this comment.
Looks good to me and I agree with all suggestions from @eddiebergman
| @@ -50,7 +50,8 @@ class TestSMBO(unittest.TestCase): | |||
| def setUp(self): | |||
| self.scenario = Scenario({'cs': test_helpers.get_branin_config_space(), | |||
| 'run_obj': 'quality', | |||
| 'output_dir': 'data-test_smbo'}) | |||
| 'output_dir': 'data-test_smbo', | |||
| "runcount-limit": 5}) | |||
| self.output_dirs = [] | |||
|
@renesass Would you have any suggestions on where to document this? I'd be happy to merge it functionally but I feel like this functionality will get lost and forgotten about unless it's written prominently in the docs somewhere. My suggestion would be to make a new Creating a dedicated file and manual section for it also means less barrier to modifying the docs for this in the future. |
|
I'd suggest to add "Advanced Usage" below "Basic Usage" under the manual tree. We can add a small example in this file for the early stopping scenario too. In the future, we can always update this file instead of creating others. |
|
Did we ever add documentation on this? Sorting out autosklearn issues and it would be good to be able to link to any documentation on this functionality? |
|
We didn't. |
|
Cool no problem, we have it as an issue, we will bug you about it when we deal with it. |
I wanted to implement a hook in autosklearn for the user to be able to implement his own stopping strategy. @eddiebergman suggested here to use the
IncorporateRunResultCallbackof SMAC. However, currently the callback cannot interrupt the optimization loop.I now added the possibility to gracefully abort the optimization by returning
Falsefrom the callback (when returningNonethe optimization still continues for backward compatibility). I also added a unit test to test this.Please let me know if there is anything I should change/add/fix. I'm very happy to contribute :)