Calibrators catalog introduction #2766

Ivanidzo4ka · 2019-02-27T20:55:26Z

should fix #1871

TomFinley · 2019-02-27T21:29:08Z

src/Microsoft.ML.Data/TrainCatalog.cs

+        /// </summary>
+        public CalibratorsSubCatalog Calibrators { get; }
+
+        public sealed class CalibratorsSubCatalog : CatalogInstantiatorBase


CalibratorsSubCatalog [](start = 28, length = 21)

If I look at other classes derived from CatalogInstantiatorBase, I don't see anything like SubCatalog. How about just CalibratorsCatalog? Why do we need the sub in there?

Don't forget to like and sub. #Resolved

TomFinley · 2019-02-27T21:32:15Z

src/Microsoft.ML.Data/Prediction/CalibratorCatalog.cs

+
+namespace Microsoft.ML
+{
+    public static class CalibratorsCatalog


CalibratorsCatalog [](start = 24, length = 18)

So, maybe we can just have CalibratorsCatalog be a class, with actual methods, that is, not extension methods, given that it's in the same assembly. We sometimes do extension methods, but we often also put methods directly on the thing when the "scope" of what is to be done is fairly limited, or there isn't some great question of broader consistency, or somesuch. #Resolved

by saying that you mean something like mlContext.Calirbrators.Platt() or you mean CalibratorsCatalog.Platt(MlContext context, ...)
Second feels outlier to way how we currently handle everything else. #Resolved

No. I am saying that these should be actual methods on the catalog class itself. #Resolved

Let me be more explicit. I could write the following code:

public class Foo { } public static class FooExtensions { public static void Bar(this Foo foo, ...) { ... } public static void Biz(this Foo foo, ...) { ... } public static void Blam(this Foo foo, ...) { ... } }

As in fact we do when we want to have separation between multiple things. But, in this particular case, everything we'd want to do with this catalog lives in the same assembly where the assembly is. So I could have written the following.

public class Foo { public void Bar(...) { ... } public void Biz(...) { ... } public void Blam(...) { ... } }

I would argue the second is easier and more clear. I understand the reasons why we sometimes phrase these catalog operations as extension methods, what I'm saying is, in this case, it is not necessary or helpful to do so. If you disagree that's fine I guess, it just seems strange. #Resolved

Thank you. I'm sorry you had to write down this explanation for me. I've been slightly tired lately and with all other distractions it looks like I lost ability to focus and understand other people. #Resolved

Ha ha, that's fine Ivan. I remember yesterday when I was so exhausted and sick I confused the heck out of people with what were (if I look back on them) some insane ramblings. As I perhaps do even when I am whole and well, though I hope not. 😄 #Resolved

codecov · 2019-02-28T01:19:24Z

Codecov Report

Merging #2766 into master will decrease coverage by 0.03%.
The diff coverage is 35%.

@@            Coverage Diff             @@
##           master    #2766      +/-   ##
==========================================
- Coverage    71.7%   71.66%   -0.04%     
==========================================
  Files         809      809              
  Lines      142491   142433      -58     
  Branches    16116    16120       +4     
==========================================
- Hits       102173   102078      -95     
- Misses      35885    35923      +38     
+ Partials     4433     4432       -1

Flag	Coverage Δ
#Debug	`71.66% <35%> (-0.04%)`	⬇️
#production	`67.9% <31.57%> (-0.03%)`	⬇️
#test	`85.86% <100%> (-0.04%)`	⬇️

Impacted Files	Coverage Δ
src/Microsoft.ML.Data/Prediction/Calibrator.cs	`72.67% <ø> (ø)`	⬆️
.../Microsoft.ML.Data/Prediction/CalibratorCatalog.cs	`90.71% <100%> (ø)`	⬆️
...ML.Tests/TrainerEstimators/CalibratorEstimators.cs	`100% <100%> (ø)`	⬆️
src/Microsoft.ML.Data/TrainCatalog.cs	`85.71% <23.52%> (-4.37%)`	⬇️
src/Microsoft.ML.TensorFlow/TensorflowCatalog.cs	`83.33% <0%> (-16.67%)`	⬇️
src/Microsoft.ML.Data/Transforms/CatalogUtils.cs	`85.71% <0%> (-14.29%)`	⬇️
...rosoft.ML.TensorFlow/TensorFlow/TensorflowUtils.cs	`57.92% <0%> (-7.45%)`	⬇️
src/Microsoft.ML.Transforms/GcnTransform.cs	`75.78% <0%> (-1.8%)`	⬇️
...c/Microsoft.ML.CpuMath/CpuMathUtils.netstandard.cs	`93.4% <0%> (-0.35%)`	⬇️
...oft.ML.Transforms/Text/TextFeaturizingEstimator.cs	`88.73% <0%> (-0.28%)`	⬇️
... and 48 more

sfilipi · 2019-02-28T05:39:30Z

src/Microsoft.ML.Data/TrainCatalog.cs

+        /// </summary>
+        public CalibratorsCatalog Calibrators { get; }
+
+        public sealed class CalibratorsCatalog : CatalogInstantiatorBase


CalibratorsCatalog [](start = 28, length = 18)

i need xml doc too. #Pending

To be honest none of other CatalogInstantiatorBase inherited classes has one. And I don't plan to fix it right now. Documentation can wait till next month.

In reply to: 261052357 [](ancestors = 261052357)

sfilipi · 2019-02-28T05:39:54Z

src/Microsoft.ML.Data/TrainCatalog.cs

+            {
+            }
+            /// <summary>
+            /// Adds probability column by training naive binning-based calbirator.


calbirator [](start = 72, length = 10)

spelling here and below. #Resolved

sfilipi · 2019-02-28T05:41:30Z

src/Microsoft.ML.Data/TrainCatalog.cs

+            {
+            }
+            /// <summary>
+            /// Adds probability column by training naive binning-based calbirator.


naive binning-based [](start = 52, length = 19)

can we link to any online resources? @yaeldekel, does this apply: https://www.ncbi.nlm.nih.gov/pmc/articles/PMC4410090/

I doubt they're the same. That article seems to have been published in 2015, whereas this calibrator has AFAIK existed at least since 2013, and probably goes back a fair bit further than that. I imagine Misha didn't just conjure up the method out of absolute thin air, but whatever work he based the implementation on has probably be lost to the sands of time.

In reply to: 261052618 [](ancestors = 261052618)

sfilipi · 2019-02-28T05:42:11Z

src/Microsoft.ML.Data/TrainCatalog.cs

+                return new NaiveCalibratorEstimator(Owner.Environment, labelColumnName, scoreColumnName);
+            }
+            /// <summary>
+            /// Adds probability column by training platt calbirator.


training platt [](start = 43, length = 14)

https://en.wikipedia.org/wiki/Platt_scaling link #Resolved

sfilipi · 2019-02-28T05:52:35Z

docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/BinaryClassification/Calibrators/Platt.cs

+            // Let's train a calibrator estimator on this scored dataset. The trained calibrator estimator produces a transformer
+            // that can transform the scored data by adding a new column names "Probability". 
+            var calibratorEstimator = mlContext.BinaryClassification.Calibrators.Platt();
+            var calibratorTransformer = calibratorEstimator.Fit(scoredData);


scoredData [](start = 64, length = 10)

we should probably train the calibrator on another dataset, not the testSet, in those samples. So have training set, testing set, and a calibrator training set

sfilipi · 2019-02-28T05:53:08Z

...rosoft.ML.Samples/Dynamic/Trainers/BinaryClassification/Calibrators/PairAdjacentViolators.cs

+using System;
+using System.Linq;
+
+namespace Microsoft.ML.Samples.Dynamic.Trainers.BinaryClassification.Calibrators


Microsoft.ML.Samples.Dynamic.Trainers.BinaryClassification.Calibrators [](start = 10, length = 70)

please Microsoft.ML.Samples.Dynamic

no need to be too fancy #Pending

I don't know. I told same thing to @shmoradims and he told me I'm silly. I don't want be part of any fights regarding samples and how we organize them. So I would just follow what he is doing.

In reply to: 261054779 [](ancestors = 261054779)

sfilipi

Ivanidzo4ka · 2019-03-01T23:54:02Z

@TomFinley would be nice if you can take a look one more time, thx!

TomFinley · 2019-03-04T20:54:44Z

src/Microsoft.ML.Data/Prediction/CalibratorCatalog.cs


    }

    /// <summary>
-    /// The <see cref="ITransformer"/> implementation obtained by training a <see cref="PavCalibratorEstimator"/>
+    /// The <see cref="ITransformer"/> implementation obtained by training a <see cref="PoolAdjacentViolatorsCalibratorEstimator"/>


PoolAdjacentViolatorsCalibratorEstimator [](start = 88, length = 40)

What is the difference between PoolAdjacentViolators and PairAdjacentViolators? They both seem to be an expansion of the single acronym that used to just be PAV, so I'm wondering if this distinction is on purpose, or if it's a mistake or something. #Resolved

Changed to Isotonic to avert confusion

In reply to: 262238146 [](ancestors = 262238146)

TomFinley · 2019-03-04T21:00:48Z

public sealed class PavCalibrator : ICalibrator, ICanSaveInBinaryFormat

So, I see we have this PAV calibrator that are I guess the model parameters from pair-adjacent calibrator transformers which are in turn trained from pool adjacent calibrator estimators? Right? I kind of feel like consistency here would be less confusing. #Resolved

Refers to: src/Microsoft.ML.Data/Prediction/Calibrator.cs:1801 in 553aeb1. [](commit_id = 553aeb1, deletion_comment = False)

TomFinley

Thank you @Ivanidzo4ka !

Ivan Matantsev added 3 commits February 26, 2019 14:24

Bu

c36487a

Merge branch 'master' into Ivanidze/CalibratorsCatalog

87f57ba

Calibrators Catalog

3d08524

TomFinley reviewed Feb 27, 2019

View reviewed changes

Ivan Matantsev added 3 commits February 27, 2019 16:01

Merge branch 'master' into Ivanidze/CalibratorsCatalog

61aea78

Merge branch 'master' into Ivanidze/CalibratorsCatalog

658cf94

Direct Calibrator catalog

0f5e386

sfilipi reviewed Feb 28, 2019

View reviewed changes

sfilipi approved these changes Feb 28, 2019

View reviewed changes

Ivan Matantsev added 3 commits February 28, 2019 14:03

pr comments

c6c4e08

fix build

959a32b

revert program.cs

553aeb1

Ivanidzo4ka requested review from yaeldekel and wschin March 4, 2019 00:32

TomFinley reviewed Mar 4, 2019

View reviewed changes

Ivan Matantsev added 3 commits March 4, 2019 13:10

Merge branch 'master' into Ivanidze/CalibratorsCatalog

69ed1c1

PAV->Isotonic

7c4db19

no more PAV

2e06d06

TomFinley approved these changes Mar 4, 2019

View reviewed changes

TomFinley merged commit 86d5dda into dotnet:master Mar 5, 2019

artidoro mentioned this pull request Mar 5, 2019

Hide more of Microsoft.ML.Data #2842

Merged

ghost locked as resolved and limited conversation to collaborators Mar 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Calibrators catalog introduction #2766

Calibrators catalog introduction #2766

Ivanidzo4ka commented Feb 27, 2019 •

edited

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 27, 2019 •

edited

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 27, 2019 •

edited

Loading

TomFinley Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

codecov bot commented Feb 28, 2019 •

edited

Loading

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 28, 2019

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

sfilipi Feb 28, 2019

TomFinley Feb 28, 2019

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

sfilipi Feb 28, 2019

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 28, 2019

sfilipi left a comment

Ivanidzo4ka commented Mar 1, 2019

TomFinley Mar 4, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Mar 4, 2019

TomFinley commented Mar 4, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley left a comment

Calibrators catalog introduction #2766

Calibrators catalog introduction #2766

Conversation

Ivanidzo4ka commented Feb 27, 2019 • edited Loading

TomFinley Feb 27, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

TomFinley Feb 27, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

TomFinley Feb 27, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

TomFinley Feb 27, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

TomFinley Feb 28, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

codecov bot commented Feb 28, 2019 • edited Loading

Codecov Report

sfilipi Feb 28, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 28, 2019

Choose a reason for hiding this comment

sfilipi Feb 28, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

sfilipi Feb 28, 2019

Choose a reason for hiding this comment

TomFinley Feb 28, 2019

Choose a reason for hiding this comment

sfilipi Feb 28, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

sfilipi Feb 28, 2019

Choose a reason for hiding this comment

sfilipi Feb 28, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 28, 2019

Choose a reason for hiding this comment

sfilipi left a comment

Choose a reason for hiding this comment

Ivanidzo4ka commented Mar 1, 2019

TomFinley Mar 4, 2019 • edited by Ivanidzo4ka Loading

Choose a reason for hiding this comment

Ivanidzo4ka Mar 4, 2019

Choose a reason for hiding this comment

TomFinley commented Mar 4, 2019 • edited by Ivanidzo4ka Loading

TomFinley left a comment

Choose a reason for hiding this comment

Ivanidzo4ka commented Feb 27, 2019 •

edited

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 27, 2019 •

edited

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley Feb 27, 2019 •

edited by Ivanidzo4ka

Loading

Ivanidzo4ka Feb 27, 2019 •

edited

Loading

TomFinley Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

codecov bot commented Feb 28, 2019 •

edited

Loading

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

sfilipi Feb 28, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley Mar 4, 2019 •

edited by Ivanidzo4ka

Loading

TomFinley commented Mar 4, 2019 •

edited by Ivanidzo4ka

Loading