Skip to content

Follow up on Calibrator estimators #1871

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

Closed
sfilipi opened this issue Dec 13, 2018 · 6 comments · Fixed by #2766
Closed

Follow up on Calibrator estimators #1871

sfilipi opened this issue Dec 13, 2018 · 6 comments · Fixed by #2766
Assignees
Labels
API Issues pertaining the friendly API documentation Related to documentation of ML.NET
Milestone

Comments

@sfilipi
Copy link
Member

sfilipi commented Dec 13, 2018

There are a few follow ups to the work to create calibrator estimators:

1- The public classes in Microsoft.ML.Core/Prediction/Calibrator.cs need to have descriptive XML documentation.
2- The Calibrators need to be added as property to the BinaryClassificationContext
3- Some of the Calibrators have a public constructor, that allows initializing the parameters. Does it make sense to expose and wire those parameters to the CalibratorEstimators, and set them from there?
So they will all look like FixedPalttCalibratorEstimator

cc @Zruty0

@sfilipi sfilipi self-assigned this Dec 13, 2018
@sfilipi sfilipi added API Issues pertaining the friendly API documentation Related to documentation of ML.NET labels Dec 13, 2018
@sfilipi sfilipi added this to the 1218 milestone Dec 13, 2018
@sfilipi sfilipi changed the title The documentation of the Calibrators, calibrator estimators needs to be written on imporved The documentation of the Calibrators, calibrator estimators needs to be written on improved Dec 13, 2018
@sfilipi sfilipi changed the title The documentation of the Calibrators, calibrator estimators needs to be written on improved Follow up on Calibrator estimators Dec 14, 2018
@Zruty0
Copy link
Contributor

Zruty0 commented Dec 14, 2018

There are 2 cases of calibrators: one where we set the parameters (slope, offset) manually, and one where we learn them.

Both cases will need an estimator: first one will just be a regular trainer, and the second will be a trivial estimator. I world prefer these 2 cases to be invoked via 2 different methods, instead of a single overload.

@TomFinley
Copy link
Contributor

TomFinley commented Feb 12, 2019

It seems to me that these when fit, given that they're parameterized with model parameters, should be producing a model with CalibratedModelParametersBase, with the sub-model being the input model. I do not see that the estimators currently are doing this.

I am at least confident we should not have two mechanisms for doing this. Right now we have two, and I'm not sure why:

  1. CalibratorTransformer<TCalibrator>, and

  2. BinaryPredictionTransformer<CalibratedModelParametersBase<TModel, TCalibrator>>

@Ivanidzo4ka
Copy link
Contributor

I can be wrong, but for me it looks like done issue.
#2601 does 1) and 3) and I believe @wschin is done with making sure we return calibrated or not calibrated model.

@TomFinley
Copy link
Contributor

TomFinley commented Feb 21, 2019

I can be wrong, but for me it looks like done issue.
#2601 does 1) and 3)

Hi @Ivanidzo4ka , you are omitting 2 I think. The estimator for calibration is still not accessible through MLContext, the IHostEnvironment constructor is still public at the present time, as we see below.

public PlattCalibratorEstimator(IHostEnvironment env,
string labelColumn = DefaultColumnNames.Label,
string scoreColumn = DefaultColumnNames.Score,
string weightColumn = null) : base(env, new PlattCalibratorTrainer(env), labelColumn, scoreColumn, weightColumn)

There is still not Calibrator's property that is some sort of CalibratorsCatalog accessible through BinaryClassification. Calibrated models are great, but we also just want to be able to calibrate scores as these estimators do (per @sfilipi), and we haven't finished writing that catalog yet.

@Ivanidzo4ka
Copy link
Contributor

Ok, let's at least add it to Project 13, otherwise it has chance to be missed.

@Ivanidzo4ka Ivanidzo4ka assigned Ivanidzo4ka and unassigned sfilipi Feb 22, 2019
@Ivanidzo4ka
Copy link
Contributor

Just to make clear.
We want something like this:

            var pipeline = mlContext.BinaryClassification.Trainers.StochasticDualCoordinateAscentNonCalibrated(
                labelColumn: "Label", featureColumn: "Features", loss: new HingeLoss(), l2Const: 0.001f)
                .Append(mlContext.BinaryClassification.Calibrators.PairAdjacentViolators("Label"));

            // Step 3: Train the pipeline created.
            var model = pipeline.Fit(data);
           // Get submodel and calibrator.
            var subModel = model.LastTransformer.Model.SubModel;
            var calibrator = model.LastTransformer.Model.Calibrator;

Or something else?

Right now CalibratorEstimator returns

public abstract class CalibratorTransformer<TICalibrator> : RowToRowTransformerBase, ISingleFeaturePredictionTransformer<TICalibrator>

which is quite blank.
image

@shauheen shauheen added this to the 0319 milestone Mar 5, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API documentation Related to documentation of ML.NET
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants