Adding binaryclassification bin score evaluator #119

gokulsfdc · 2018-09-07T12:18:03Z

Related issues
#101

Describe the proposed solution
Creating a evaluator for Binary Classification which provides statistics about the predicted scores. This evaluator creates the specified number of bins and computes the following details for each bin.

Total number of data points.
Average Score.
Average Conversion rate.
Bin Centers.

Overall BrierScore for the scores is also computed, which is a default metric as well.

Added unit tests for the evaluator

tovbinm · 2018-09-07T13:18:25Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+ */
+case class BinaryClassificationBinMetrics
+(
+  BrierScore: Double,


Names should start with a lower case: brierScore

So for some reason all of our metrics do not follow this convention @tovbinm. We should figure out what we want and then make them all consistent.

tovbinm · 2018-09-07T13:18:58Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+  val predictionLabel = "pred"
+
+  val dataset_test = Seq(
+    (Map("probability_1" -> 0.99999, "probability_0" -> 0.0001, "prediction" -> 1.0), 1.0),


Use Prediction type instead

tovbinm · 2018-09-07T13:21:37Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+@RunWith(classOf[JUnitRunner])
+class OpBinaryClassifyBinEvaluatorTest extends FlatSpec with TestSparkContext {
+
+  val labelName = "label"


You can create a dataframe using TestFeatureBuilder easily. That way you need to define these strings here and create dataframes with spark below in tests.

tovbinm · 2018-09-07T13:26:07Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+case class BinaryClassificationBinMetrics
+(
+  BrierScore: Double,
+  BinCenters: Seq[Double],


Json annotations has to added on sequences. See other binclass eval metrics class.

tovbinm · 2018-09-07T13:27:48Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+ * @param name            name of default metric
+ * @param isLargerBetter  is metric better if larger
+ * @param uid             uid for instance
+ */


Perhaps rename to OpBinScoreEvaluator?

Also this evaluator has to be added to the Evaluators factory object as well and BinaryClassEvalMetrics enum.

This evaluator returns 5 diff values. Should there be 5 factory methods? or only for Brier Score, which is the defaultmetric?

only the Brier Score. @leahmcguire wdyt?

yes only for the brier score - the other metrics are support for the brier score. The brier score is the only metric that could be used for optimization

tovbinm · 2018-09-07T15:01:31Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+    val metrics = new OpBinaryClassifyBinEvaluator(numBins = 0)
+      .setLabelCol(labelName).setPredictionCol(predictionLabel).evaluateAll(df)
+
+    metrics.BrierScore shouldBe 0.0


case classes has equals implemented so you can simply do metrics shouldBe BinaryClassificationBinMetrics(0.0, Seq(), Seq(), Seq(), Seq()) here and everywhere in tests

tovbinm · 2018-09-07T15:02:16Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+    metrics.AverageConversionRate shouldBe Seq(0.0, 0.0, 0.0, 1.0)
+  }
+
+  it should "return the empty bin metrics for numBins == 0" in {


it should "error on invalid num of bins"

tovbinm · 2018-09-07T15:02:45Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+    metrics.AverageConversionRate shouldBe Seq.empty[Double]
+  }
+
+  it should "return the bin metrics for skewed data" in {


it should "evaluate bin metrics for skewed data"

tovbinm · 2018-09-07T15:03:12Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+    metrics.AverageConversionRate shouldBe Seq(0.0, 0.0, 0.0, 0.0, 1.0)
+  }
+
+  it should "return the default metric as BrierScore" in {


it should "evaluate the default metric as BrierScore"

tovbinm · 2018-09-07T15:03:31Z

core/src/test/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluatorTest.scala

+
+  val emptyDataSet = Seq.empty[(Map[String, Double], Double)]
+
+  Spec[OpBinaryClassifyBinEvaluator] should "return the bin metrics" in {


should "evaluate the bin metrics"

codecov · 2018-09-08T12:21:02Z

Codecov Report

Merging #119 into master will increase coverage by 0.02%.
The diff coverage is 93.1%.

@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   86.19%   86.22%   +0.02%     
==========================================
  Files         298      299       +1     
  Lines        9668     9696      +28     
  Branches      329      542     +213     
==========================================
+ Hits         8333     8360      +27     
- Misses       1335     1336       +1

Impacted Files	Coverage Δ
...com/salesforce/op/evaluators/OpEvaluatorBase.scala	`90.47% <ø> (ø)`	⬆️
...c/main/scala/com/salesforce/op/ModelInsights.scala	`94.11% <ø> (ø)`	⬆️
...cala/com/salesforce/op/evaluators/Evaluators.scala	`97.14% <100%> (+0.08%)`	⬆️
...sification/BinaryClassificationModelSelector.scala	`95.65% <100%> (ø)`	⬆️
...op/stages/impl/selector/ModelSelectorSummary.scala	`91.3% <66.66%> (-0.84%)`	⬇️
...salesforce/op/evaluators/OpBinScoreEvaluator.scala	`95.65% <95.65%> (ø)`
...om/salesforce/op/utils/spark/OpSparkListener.scala	`98.7% <0%> (+1.29%)`	⬆️

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 078ea62...4b32250. Read the comment docs.

leahmcguire · 2018-09-08T20:28:05Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+    val dataProcessed = makeDataToUse(data, labelColumnName)
+
+    import dataProcessed.sparkSession.implicits._
+    val rdd = dataProcessed.select(getPredictionValueCol, labelColumnName).as[(Double, Double)].rdd


you dont seem to use this rdd beyond checking if the dataset is empty

leahmcguire · 2018-09-08T20:47:40Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+      } else {
+        // Find the significant digit to which the scores needs to be rounded, based of numBins.
+        val significantDigitToRoundOff = math.log10(numBins).toInt + 1
+        val scoreAndLabelsRounded = for {i <- scoreAndLabels}


I think my explanation on the doc may have made this more complicated than it needs to be.
The probabilities will be between 0 and 1 so use that information to compute the bins and their centers. Make a function binFn that takes in the probability and returns the bin. Then you can simply do:
scoreAndLabels.map{ case (score, label) => (binFun(score), (score, label, 1L) }.reduceByKey(_ + _) .map{ case (bin, (scoreSum, labelSum, count)) => (bin, scoreSum/count, labelSum/count, count) }
(you will need to import com.twitter.algebird.Operators._ for the reduceByKey to work without specifying everything)

leahmcguire · 2018-09-08T20:50:36Z

core/src/main/scala/com/salesforce/op/evaluators/OpBinaryClassifyBinEvaluator.scala

+ */
+case class BinaryClassificationBinMetrics
+(
+  BrierScore: Double,


So for some reason all of our metrics do not follow this convention @tovbinm. We should figure out what we want and then make them all consistent.

leahmcguire

LGTM! This is awesome, thank you for contributing!!

tovbinm · 2018-09-12T12:39:55Z

please take a look into the failing test

[-] BinaryClassificationModelSelector should fit and predict for default models (497.062 secs)
com.salesforce.op.stages.impl.classification.BinaryClassificationModelSelectorTest > BinaryClassificationModelSelector should fit and predict for default models FAILED
    java.lang.ArrayIndexOutOfBoundsException: -175
        at com.salesforce.op.evaluators.OpBinScoreEvaluator$$anonfun$7.apply(OpBinScoreEvaluator.scala:100)
        at com.salesforce.op.evaluators.OpBinScoreEvaluator$$anonfun$7.apply(OpBinScoreEvaluator.scala:96)
        at scala.collection.IndexedSeqOptimized$class.foldl(IndexedSeqOptimized.scala:57)
        at scala.collection.IndexedSeqOptimized$class.foldLeft(IndexedSeqOptimized.scala:66)
        at scala.collection.mutable.ArrayOps$ofRef.foldLeft(ArrayOps.scala:186)
        at com.salesforce.op.evaluators.OpBinScoreEvaluator.evaluateAll(OpBinScoreEvaluator.scala:96)
        at com.salesforce.op.evaluators.OpBinScoreEvaluator.evaluateAll(OpBinScoreEvaluator.scala:59)
        at com.salesforce.op.stages.impl.selector.HasEval$$anonfun$1.apply(ModelSelectorNames.scala:92)
        at com.salesforce.op.stages.impl.selector.HasEval$$anonfun$1.apply(ModelSelectorNames.scala:89)
        at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
        at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
        at scala.collection.immutable.List.foreach(List.scala:392)
        at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
        at scala.collection.immutable.List.map(List.scala:296)
        at com.salesforce.op.stages.impl.selector.HasEval$class.evaluate(ModelSelectorNames.scala:89)
        at com.salesforce.op.stages.impl.selector.ModelSelector.evaluate(ModelSelector.scala:74)
        at com.salesforce.op.stages.impl.selector.ModelSelector.fit(ModelSelector.scala:171)
        at com.salesforce.op.stages.impl.classification.BinaryClassificationModelSelectorTest$$anonfun$8.apply(BinaryClassificationModelSelectorTest.scala:186)
        at com.salesforce.op.stages.impl.classification.BinaryClassificationModelSelectorTest$$anonfun$8.apply(BinaryClassificationModelSelectorTest.scala:171)

https://travis-ci.com/salesforce/TransmogrifAI/jobs/145162156

tovbinm

there is actually a deeper issue with probability values, so I actually let @leahmcguire comment here.

leahmcguire · 2018-09-12T20:28:00Z

Yeah so what is happening is that not every model type produces a probablity column. And there is a special case in the makeData call that will fall back to the rawPrediciton value instead of the probability. This is not bounded so you will actually need to do a reduce to get the min and max values for the bin range.

gokulsfdc · 2018-09-13T02:51:11Z

So do you mean, default min max value to compute bin should be [0,1]
If probability goes out of bounds due to the fallback , then this range needs to be altered accordingly

…bin_evaluator

salesforce-cla · 2020-11-25T20:49:52Z

Thanks for the contribution! It looks like @leahmcguire is an internal user so signing the CLA is not required. However, we need to confirm this.

salesforce-cla · 2021-03-10T00:57:42Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Leah McGuire <l***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

creating binaryclassification bin score evaluator

2d9cb1d

gokulsfdc requested review from leahmcguire and tovbinm as code owners September 7, 2018 12:18

salesforce-cla bot added the cla:signed label Sep 7, 2018

gokulsfdc added 3 commits September 7, 2018 17:52

minor changes

9658eb4

Doc Changes

56d6280

Fixing code factor issue

0eebe3d

tovbinm requested changes Sep 7, 2018

View reviewed changes

Incorporating code review comments

41123d5

leahmcguire reviewed Sep 8, 2018

View reviewed changes

gokulsfdc added 4 commits September 9, 2018 22:48

code review comments

a16fc86

removing partitioner

ee8924f

code review comments

919b933

fixing testbuilder for tests

b421d33

leahmcguire approved these changes Sep 11, 2018

View reviewed changes

leahmcguire and others added 5 commits September 11, 2018 09:14

Merge branch 'master' into op/bin_evaluator

b5a70ee

Merge branch 'master' into op/bin_evaluator

9e613ed

computing stats efficiently

728aa99

unfolding the tuples

db653bf

code review comments

6ee1a82

Fixing the stats when score goes out of bound

4b32250

tovbinm approved these changes Sep 12, 2018

View reviewed changes

tovbinm requested changes Sep 12, 2018

View reviewed changes

gokulsfdc and others added 3 commits September 13, 2018 10:26

Merge branch 'master' of github.com:salesforce/TransmogrifAI into op/…

483c937

…bin_evaluator

fixing an issue where prediction column is missing

d928ba7

Merge branch 'master' into op/bin_evaluator

01c89eb

gokulsfdc added 2 commits September 14, 2018 07:12

finding max and min using foldleft

b36ec1d

Finding min max using map-fold

01250d2

tovbinm approved these changes Sep 14, 2018

View reviewed changes

tovbinm merged commit 7ba2a02 into salesforce:master Sep 14, 2018

gokulsfdc deleted the op/bin_evaluator branch September 14, 2018 17:06

ericwayman pushed a commit that referenced this pull request Feb 8, 2019

Adding binaryclassification bin score evaluator (#119)

3614a57

salesforce-cla bot added cla:missing and removed cla:signed labels Nov 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding binaryclassification bin score evaluator #119

Adding binaryclassification bin score evaluator #119

gokulsfdc commented Sep 7, 2018 •

edited

Loading

tovbinm Sep 7, 2018

leahmcguire Sep 8, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

gokulsfdc Sep 7, 2018

tovbinm Sep 7, 2018

leahmcguire Sep 8, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

tovbinm Sep 7, 2018

codecov bot commented Sep 8, 2018 •

edited

Loading

leahmcguire Sep 8, 2018

leahmcguire Sep 8, 2018

leahmcguire Sep 8, 2018

leahmcguire left a comment

tovbinm commented Sep 12, 2018 •

edited

Loading

tovbinm left a comment

leahmcguire commented Sep 12, 2018

gokulsfdc commented Sep 13, 2018

salesforce-cla bot commented Nov 25, 2020

salesforce-cla bot commented Mar 10, 2021


		val emptyDataSet = Seq.empty[(Map[String, Double], Double)]

		Spec[OpBinaryClassifyBinEvaluator] should "return the bin metrics" in {

Adding binaryclassification bin score evaluator #119

Adding binaryclassification bin score evaluator #119

Conversation

gokulsfdc commented Sep 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 8, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leahmcguire left a comment

Choose a reason for hiding this comment

tovbinm commented Sep 12, 2018 • edited Loading

tovbinm left a comment

Choose a reason for hiding this comment

leahmcguire commented Sep 12, 2018

gokulsfdc commented Sep 13, 2018

salesforce-cla bot commented Nov 25, 2020

salesforce-cla bot commented Mar 10, 2021

gokulsfdc commented Sep 7, 2018 •

edited

Loading

codecov bot commented Sep 8, 2018 •

edited

Loading

tovbinm commented Sep 12, 2018 •

edited

Loading