-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add callback for error handling in AutoMLSearch #1403
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1403 +/- ##
=========================================
+ Coverage 100.0% 100.0% +0.1%
=========================================
Files 213 214 +1
Lines 13975 14040 +65
=========================================
+ Hits 13968 14033 +65
Misses 7 7
Continue to review full report at Codecov.
|
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.
Looks good! I agree with @freddyaboulton that we should add the traceback as part of saving the errors. Could be done in this PR or another but would be great for looking glass.
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.
@angela97lin this looks great!
I left a few comments, but there was nothing major.
RE Protocol
subclassing, I think we're ok to skip that for now, but its a good idea.
evalml/automl/automl_search.py
Outdated
@@ -674,22 +680,14 @@ def _compute_cv_scores(self, pipeline, X, y): | |||
logger.debug(f"\t\t\tFold {i}: {self.objective.name} score: {scores[self.objective.name]:.3f}") | |||
score = scores[self.objective.name] | |||
except Exception as e: | |||
if self.error_callback is not None: | |||
self.error_callback(e, self, fold_num=i, pipeline=pipeline) |
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.
Cool, so because we pass self
back in the callback, someone could theoretically write a callback which responds to failure and calls automl.search
again. So this is implementing one part of an asynchronous API (#1047 ). Neat!
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.
Yes!!
Closes #1217
Questions:
If all objectives fail and
AutoMLException
is raised, we will raise since it indicates that something is likely wrong. This is the same behavior as what is currently implemented.