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

Evaluators check for empty data #178

Merged
merged 17 commits into from
Nov 9, 2018
Merged

Evaluators check for empty data #178

merged 17 commits into from
Nov 9, 2018

Conversation

tovbinm
Copy link
Collaborator

@tovbinm tovbinm commented Nov 7, 2018

Related issues
Most of our evaluators never check for empty during evaluate call and fail miserably.

Describe the proposed solution

  • Added checks for empty data in Evaluators factory, which return defaults in case of no data was provided
  • DRY out Evaluators factory
  • Update EvaluatorsTest to use property based testing with Tables. Each evaluators is now being evaluated against non empty and an empty dataset.
  • Fix evaluation metrics order in ModelInsights pretty print
  • Camel case Brier Score from brierScore -> BrierScore

Describe alternatives you've considered
NA

Additional context
Follows up on PR #176

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #178 into master will decrease coverage by <.01%.
The diff coverage is 87.17%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
- Coverage   86.33%   86.33%   -0.01%     
==========================================
  Files         305      307       +2     
  Lines        9976     9971       -5     
  Branches      343      542     +199     
==========================================
- Hits         8613     8608       -5     
  Misses       1363     1363
Impacted Files Coverage Δ
...orce/op/stages/impl/tuning/OpCrossValidation.scala 97.67% <ø> (ø) ⬆️
...com/salesforce/op/evaluators/OpEvaluatorBase.scala 92.59% <ø> (+2.11%) ⬆️
...alesforce/op/stages/impl/evaluator/OPLogLoss.scala 69.23% <100%> (ø) ⬆️
...cala/com/salesforce/op/evaluators/Evaluators.scala 100% <100%> (+2.89%) ⬆️
...salesforce/op/evaluators/OpBinScoreEvaluator.scala 96.29% <100%> (-0.14%) ⬇️
...lesforce/op/evaluators/OpRegressionEvaluator.scala 91.66% <100%> (ø) ⬆️
.../com/salesforce/op/utils/spark/RichEvaluator.scala 100% <100%> (ø)
...op/evaluators/OpMultiClassificationEvaluator.scala 94.73% <33.33%> (ø) ⬆️
...p/evaluators/OpBinaryClassificationEvaluator.scala 82.5% <66.66%> (+0.92%) ⬆️
...c/main/scala/com/salesforce/op/ModelInsights.scala 92.62% <72.72%> (+0.37%) ⬆️
... and 5 more

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 76cd1e3...790c2ea. Read the comment docs.

@tovbinm tovbinm changed the title Evaluators empty check Evaluators check for empty data Nov 8, 2018
@tovbinm tovbinm merged commit cdfa7f4 into master Nov 9, 2018
@tovbinm tovbinm deleted the mt/evaluators-empty-check branch November 9, 2018 18:57
ericwayman pushed a commit that referenced this pull request Feb 8, 2019
@tovbinm tovbinm mentioned this pull request Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants