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
Update AutoML to use objective decision function during scoring for custom objectives #1934
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1934 +/- ##
=========================================
+ Coverage 100.0% 100.0% +0.1%
=========================================
Files 273 274 +1
Lines 22381 22418 +37
=========================================
+ Hits 22375 22412 +37
Misses 6 6
Continue to review full report at Codecov.
|
predictions = self._predict_with_objective(X, ypred_proba, objective) | ||
return infer_feature_types(predictions) | ||
|
||
def _predict_with_objective(self, X, ypred_proba, objective): |
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.
Helper function that takes in predict_proba
, so we don't have to recalculate it for each objective.
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. Left a question for my understanding, but implementation looks good to me
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.
I think there's something funky with the double anys on L77 of binary_classification_pipeline.py. Besides that, this looks pretty solid.
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 good to me! I left some scattered comments throughout. One thing that would be nice to address before merge is adding a time series test to test_binary_predict_pipeline_use_objective
problem_type = ProblemTypes.BINARY | ||
|
||
@property |
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.
Moved all of this to BinaryClassificationPipelineMixin
@patch('evalml.pipelines.MulticlassClassificationPipeline.fit') | ||
@patch('evalml.pipelines.MulticlassClassificationPipeline.score') | ||
@patch('evalml.pipelines.MulticlassClassificationPipeline.predict') | ||
def test_pipeline_thresholding_errors(mock_multi_predict, mock_multi_score, mock_multi_fit, |
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.
Removed multiclass tests and moved to test_binary_classification_pipelines.py
Closes #1868
BinaryClassificationPipelineMixin
to share logic between binary classification / time series binary classification pipelines regarding the pipeline threshold.optimize_threshold
from ClassificationPipeline to BinaryClassificationPipelineMixin. I believe this makes sense since only binary classification pipelines, not multiclass classification pipelines, know about thresholds.