Skip to content
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

Use range-based approach for high variance check in AutoMLSearch #2622

Merged
merged 11 commits into from Aug 18, 2021

Conversation

bchen1116
Copy link
Contributor

fix #2621
Design doc and discussion here

@bchen1116 bchen1116 self-assigned this Aug 12, 2021
@codecov
Copy link

codecov bot commented Aug 12, 2021

Codecov Report

Merging #2622 (f5a1a7f) into main (869cbaa) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2622     +/-   ##
=======================================
+ Coverage   99.9%   99.9%   +0.1%     
=======================================
  Files        298     298             
  Lines      27098   27161     +63     
=======================================
+ Hits       27054   27117     +63     
  Misses        44      44             
Impacted Files Coverage Δ
evalml/automl/automl_search.py 99.9% <100.0%> (+0.1%) ⬆️
evalml/objectives/cost_benefit_matrix.py 100.0% <100.0%> (ø)
evalml/objectives/fraud_cost.py 100.0% <100.0%> (ø)
evalml/objectives/lead_scoring.py 100.0% <100.0%> (ø)
evalml/objectives/objective_base.py 100.0% <100.0%> (ø)
evalml/objectives/sensitivity_low_alert.py 100.0% <100.0%> (ø)
evalml/objectives/standard_metrics.py 100.0% <100.0%> (ø)
evalml/tests/automl_tests/test_automl.py 99.7% <100.0%> (+0.1%) ⬆️
evalml/tests/conftest.py 98.6% <100.0%> (+0.1%) ⬆️
evalml/tests/objective_tests/test_objectives.py 100.0% <100.0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 869cbaa...f5a1a7f. Read the comment docs.

@@ -369,6 +390,7 @@ class LogLossBinary(BinaryClassificationObjective):
score_needs_proba = True
perfect_score = 0.0
is_bounded_like_percentage = False # Range [0, Inf)
expected_range = [0, 1]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the range of log loss is (0, inf), we choose to set its expected range much smaller. I chose [0, 1] since we ideally expect the models to output very similar scores throughout the different CV folds. If the scores are too different (in this case, difference >= 0.5), we want to warn users that there could be high variance

if objectives == "Log Loss Binary":
assert automl._check_for_high_variance(pipeline, cv_scores)
else:
assert not automl._check_for_high_variance(pipeline, cv_scores)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This objective shouldn't raise any high variance warnings

@bchen1116 bchen1116 requested review from freddyaboulton, dsherry, angela97lin and a team and removed request for freddyaboulton, dsherry and angela97lin August 12, 2021 20:16
Copy link
Contributor

@ParthivNaresh ParthivNaresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@@ -494,6 +522,7 @@ class R2(RegressionObjective):
score_needs_proba = False
perfect_score = 1
is_bounded_like_percentage = False # Range (-Inf, 1]
expected_range = [-1, 1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing the conversation from the design doc, I think many negative R2 scores below 1 come from outliers which is what LogTransformer was hoping to address. That being said, I don't know if we're going to see similar scores across CV if outliers exist, in which case I think this will always throw high variance. Not a blocker, just something I think we should be aware of.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ParthivNaresh That's a good point! When there are outliers, this will likely raise a high variance warning more often, but I think that makes sense to do as well. In scenarios where each fold can get different enough variation of data where the models perform fairly differently, raising a high variance warning would be beneficial

evalml/tests/automl_tests/test_automl.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@chukarsten chukarsten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM! Thanks! I think I just put up a small comment that's a nit, but no big deal. Sorry, I reviewed this yesterday but somehow forgot to hit approve.

evalml/automl/automl_search.py Show resolved Hide resolved
Copy link
Contributor

@eccabay eccabay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! My only request would be to add a small section in the defining custom objectives section of our docs to reflect the addition of the expected_range.

evalml/objectives/objective_base.py Outdated Show resolved Hide resolved
@bchen1116 bchen1116 merged commit 7c81cea into main Aug 18, 2021
@chukarsten chukarsten mentioned this pull request Aug 19, 2021
@freddyaboulton freddyaboulton deleted the bc_2621_ranges branch May 13, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use range-based approach for detecting High Variance
4 participants