Skip to content

Better names to calibreated linear classification models #3034

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Mar 25, 2019
Merged

Better names to calibreated linear classification models #3034

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ba22e12
Sync names in logistic regression and maximum entropy trainers based …
wschin Mar 20, 2019
60d7d9c
Add keyword Static
wschin Mar 20, 2019
d28c5ce
Rename L-BFGS family members
wschin Mar 20, 2019
901e706
Add missing n back
wschin Mar 20, 2019
4302c88
Use SDCA-style name for LBFGS LR
wschin Mar 20, 2019
38b9940
Propose Lbfgs Logistic Regression Again
wschin Mar 22, 2019
32a5e60
Merge branch 'master' into sync-lbfgs-lr-me
wschin Mar 22, 2019
b177db1
Update EP
wschin Mar 22, 2019
804c4a8
Fix libmf
wschin Mar 22, 2019
3aadd3e
Merge branch 'master' into sync-lbfgs-lr-me
wschin Mar 25, 2019
ff16bee
Merge branch 'sync-lbfgs-lr-me' of github.com:wschin/machinelearning …
wschin Mar 25, 2019
7451306
Implement conclusions
wschin Mar 25, 2019
3dfa9cf
Update baseline because of class renaming
wschin Mar 25, 2019
7750434
Merge branch 'master' into sync-lbfgs-lr-me
wschin Mar 25, 2019
8af0d21
Fix newly-added file
wschin Mar 25, 2019
7b7cf8d
More new files..
wschin Mar 25, 2019
7c8e882
More and more new changes..
wschin Mar 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/Microsoft.ML.StandardTrainers/Standard/LogisticRegression/LogisticRegression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,21 @@
using Microsoft.ML.Runtime;
using Microsoft.ML.Trainers;

[assembly: LoadableClass(LogisticRegressionBinaryTrainer.Summary, typeof(LogisticRegressionBinaryTrainer), typeof(LogisticRegressionBinaryTrainer.Options),
[assembly: LoadableClass(LbfgsLogisticRegressionTrainer.Summary, typeof(LbfgsLogisticRegressionTrainer), typeof(LbfgsLogisticRegressionTrainer.Options),
new[] { typeof(SignatureBinaryClassifierTrainer), typeof(SignatureTrainer), typeof(SignatureFeatureScorerTrainer) },
LogisticRegressionBinaryTrainer.UserNameValue,
LogisticRegressionBinaryTrainer.LoadNameValue,
LogisticRegressionBinaryTrainer.ShortName,
LbfgsLogisticRegressionTrainer.UserNameValue,
LbfgsLogisticRegressionTrainer.LoadNameValue,
LbfgsLogisticRegressionTrainer.ShortName,
"logisticregressionwrapper")]

[assembly: LoadableClass(typeof(void), typeof(LogisticRegressionBinaryTrainer), null, typeof(SignatureEntryPointModule), LogisticRegressionBinaryTrainer.LoadNameValue)]
[assembly: LoadableClass(typeof(void), typeof(LbfgsLogisticRegressionTrainer), null, typeof(SignatureEntryPointModule), LbfgsLogisticRegressionTrainer.LoadNameValue)]

namespace Microsoft.ML.Trainers
{

/// <include file='doc.xml' path='doc/members/member[@name="LBFGS"]/*' />
/// <include file='doc.xml' path='docs/members/example[@name="LogisticRegressionBinaryClassifier"]/*' />
public sealed partial class LogisticRegressionBinaryTrainer : LbfgsTrainerBase<LogisticRegressionBinaryTrainer.Options,
public sealed partial class LbfgsLogisticRegressionTrainer : LbfgsTrainerBase<LbfgsLogisticRegressionTrainer.Options,
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Mar 20, 2019 •
edited by wschin
Loading

Choose a reason for hiding this comment

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

LbfgsLogisticRegressionTrainer [](start = 32, length = 30)

Can you call it LbfgsBinaryTrainer? #Resolved

Copy link
Member Author

@wschin wschin Mar 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It looks fine if we consider this trainer and its SDCA counterpart (they all solve LR). However, it will break the consistency between LBFGS trainers. Please take a look at Iteration 3.
LBFGS trainer names:
LbfgsLogisticRegressionTrainer
LbfgsPoissonRegressionTrainer
LbfgsMaximumEntropyTrainer

Note that we can't drop model names (such as LogisticRegression) because we need PoissonRegression to warn users that PoissonRegression is not a common case.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@sfilipi has comment on your issue, and I share it as well.
Putting word Regression without Binary in the trainer is confusing.
If I would saw name of this trainer I would assume it's part of regression task.
Which is not.

In reply to: 267475700 [](ancestors = 267475700,267460936)

Copy link
Member

@sfilipi sfilipi Mar 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

yeah, given the target user base, IMO should do a one off for this one, and keep binary in the name.

(also this feels like a classic academic vs engineering solution of a problem :D )

Also, IMO not all details need to be in the name. IMO we don't need to distingush that this is Lbfgs logistic regression through renaming. It can be in the documentations. shorter names are better...

In reply to: 267477762 [](ancestors = 267477762,267475700,267460936)

Copy link
Member Author

Choose a reason for hiding this comment

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

It will make the API looks like mlContext.BinaryClassification.LbfgsBinary(...) so I switch to SDCA-style name LbfgsCalibrated.

In reply to: 267481155 [](ancestors = 267481155,267477762,267475700,267460936)

Copy link
Member Author

@wschin wschin Mar 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

yeah, given the target user base, IMO should do a one off for this one, and keep binary in the name.

(also this feels like a classic academic vs engineering solution of a problem :D )

Also, IMO not all details need to be in the name. IMO we don't need to distingush that this is Lbfgs logistic regression through renaming. It can be in the documentations. shorter names are better...

There are several trainers returning the same model types. Only this one is called LogisticRegression and this one has been outperformed by other algorithms. Thus, I want to append Lbfgs to emphasize that this is not the only trainer for logistic regression.

In reply to: 267477762 [](ancestors = 267477762,267475700,267460936)

#Resolved

BinaryPredictionTransformer<CalibratedModelParametersBase<LinearBinaryModelParameters, PlattCalibrator>>,
CalibratedModelParametersBase<LinearBinaryModelParameters, PlattCalibrator>>
{
Expand All @@ -54,7 +54,7 @@ public sealed class Options : OptionsBase
/// <summary>
/// The instance of <see cref="ComputeLogisticRegressionStandardDeviation"/> that computes the std of the training statistics, at the end of training.
/// The calculations are not part of Microsoft.ML package, due to the size of MKL.
/// If you need these calculations, add the Microsoft.ML.Mkl.Components package, and initialize <see cref="LogisticRegressionBinaryTrainer.Options.ComputeStandardDeviation"/>.
/// If you need these calculations, add the Microsoft.ML.Mkl.Components package, and initialize <see cref="LbfgsLogisticRegressionTrainer.Options.ComputeStandardDeviation"/>.
/// to the <see cref="ComputeLogisticRegressionStandardDeviation"/> implementation in the Microsoft.ML.Mkl.Components package.
/// </summary>
public ComputeLogisticRegressionStandardDeviation ComputeStandardDeviation;
Expand All @@ -64,7 +64,7 @@ public sealed class Options : OptionsBase
private ModelStatisticsBase _stats;

/// <summary>
/// Initializes a new instance of <see cref="LogisticRegressionBinaryTrainer"/>
/// Initializes a new instance of <see cref="LbfgsLogisticRegressionTrainer"/>
/// </summary>
/// <param name="env">The environment to use.</param>
/// <param name="labelColumn">The name of the label column.</param>
Expand All @@ -73,9 +73,9 @@ public sealed class Options : OptionsBase
/// <param name="enforceNoNegativity">Enforce non-negative weights.</param>
/// <param name="l1Weight">Weight of L1 regularizer term.</param>
/// <param name="l2Weight">Weight of L2 regularizer term.</param>
/// <param name="memorySize">Memory size for <see cref="LogisticRegressionBinaryTrainer"/>. Low=faster, less accurate.</param>
/// <param name="memorySize">Memory size for <see cref="LbfgsLogisticRegressionTrainer"/>. Low=faster, less accurate.</param>
/// <param name="optimizationTolerance">Threshold for optimizer convergence.</param>
internal LogisticRegressionBinaryTrainer(IHostEnvironment env,
internal LbfgsLogisticRegressionTrainer(IHostEnvironment env,
string labelColumn = DefaultColumnNames.Label,
string featureColumn = DefaultColumnNames.Features,
string weights = null,
Expand All @@ -95,9 +95,9 @@ internal LogisticRegressionBinaryTrainer(IHostEnvironment env,
}

/// <summary>
/// Initializes a new instance of <see cref="LogisticRegressionBinaryTrainer"/>
/// Initializes a new instance of <see cref="LbfgsLogisticRegressionTrainer"/>
/// </summary>
internal LogisticRegressionBinaryTrainer(IHostEnvironment env, Options options)
internal LbfgsLogisticRegressionTrainer(IHostEnvironment env, Options options)
: base(env, options, TrainerUtils.MakeBoolScalarLabel(options.LabelColumnName))
{
_posWeight = 0;
Expand Down Expand Up @@ -127,7 +127,7 @@ private protected override BinaryPredictionTransformer<CalibratedModelParameters
=> new BinaryPredictionTransformer<CalibratedModelParametersBase<LinearBinaryModelParameters, PlattCalibrator>>(Host, model, trainSchema, FeatureColumn.Name);

/// <summary>
/// Continues the training of a <see cref="LogisticRegressionBinaryTrainer"/> using an already trained <paramref name="modelParameters"/> and returns
/// Continues the training of a <see cref="LbfgsLogisticRegressionTrainer"/> using an already trained <paramref name="modelParameters"/> and returns
/// a <see cref="BinaryPredictionTransformer{CalibratedModelParametersBase}"/>.
/// </summary>
public BinaryPredictionTransformer<CalibratedModelParametersBase<LinearBinaryModelParameters, PlattCalibrator>> Fit(IDataView trainData, LinearModelParameters modelParameters)
Expand Down Expand Up @@ -417,7 +417,7 @@ internal static CommonOutputs.BinaryClassificationOutput TrainBinary(IHostEnviro
EntryPointUtils.CheckInputArgs(host, input);

return TrainerEntryPointsUtils.Train<Options, CommonOutputs.BinaryClassificationOutput>(host, input,
() => new LogisticRegressionBinaryTrainer(host, input),
() => new LbfgsLogisticRegressionTrainer(host, input),
() => TrainerEntryPointsUtils.FindColumn(host, input.TrainingData.Schema, input.LabelColumnName),
() => TrainerEntryPointsUtils.FindColumn(host, input.TrainingData.Schema, input.ExampleWeightColumnName));
}
Expand All @@ -436,7 +436,7 @@ public abstract class ComputeLogisticRegressionStandardDeviation
/// Computes the standard deviation matrix of each of the non-zero training weights, needed to calculate further the standard deviation,
/// p-value and z-Score.
/// The calculations are not part of Microsoft.ML package, due to the size of MKL.
/// If you need these calculations, add the Microsoft.ML.Mkl.Components package, and initialize <see cref="LogisticRegressionBinaryTrainer.Options.ComputeStandardDeviation"/>
/// If you need these calculations, add the Microsoft.ML.Mkl.Components package, and initialize <see cref="LbfgsLogisticRegressionTrainer.Options.ComputeStandardDeviation"/>
/// to the <see cref="ComputeLogisticRegressionStandardDeviation"/> implementation in the Microsoft.ML.Mkl.Components package.
/// Due to the existence of regularization, an approximation is used to compute the variances of the trained linear coefficients.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion ...Microsoft.ML.StandardTrainers/Standard/LogisticRegression/MulticlassLogisticRegression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public sealed class Options : OptionsBase
/// <param name="enforceNoNegativity">Enforce non-negative weights.</param>
/// <param name="l1Weight">Weight of L1 regularizer term.</param>
/// <param name="l2Weight">Weight of L2 regularizer term.</param>
/// <param name="memorySize">Memory size for <see cref="LogisticRegressionBinaryTrainer"/>. Low=faster, less accurate.</param>
/// <param name="memorySize">Memory size for <see cref="LbfgsLogisticRegressionTrainer"/>. Low=faster, less accurate.</param>
/// <param name="optimizationTolerance">Threshold for optimizer convergence.</param>
internal LbfgsMaximumEntropyTrainer(IHostEnvironment env,
string labelColumn = DefaultColumnNames.Label,
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.ML.StandardTrainers/Standard/PoissonRegression/PoissonRegression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public sealed class Options : OptionsBase
/// <param name="l1Weight">Weight of L1 regularizer term.</param>
/// <param name="l2Weight">Weight of L2 regularizer term.</param>
/// <param name="optimizationTolerance">Threshold for optimizer convergence.</param>
/// <param name="memorySize">Memory size for <see cref="LogisticRegressionBinaryTrainer"/>. Low=faster, less accurate.</param>
/// <param name="memorySize">Memory size for <see cref="LbfgsLogisticRegressionTrainer"/>. Low=faster, less accurate.</param>
/// <param name="enforceNoNegativity">Enforce non-negative weights.</param>
internal PoissonRegressionTrainer(IHostEnvironment env,
string labelColumn = DefaultColumnNames.Label,
Expand Down
16 changes: 8 additions & 8 deletions src/Microsoft.ML.StandardTrainers/StandardTrainersCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Microsoft.ML
{
using LROptions = LogisticRegressionBinaryTrainer.Options;
using LROptions = LbfgsLogisticRegressionTrainer.Options;

/// <summary>
/// TrainerEstimator extension methods.
Expand Down Expand Up @@ -498,7 +498,7 @@ public static OnlineGradientDescentTrainer OnlineGradientDescent(this Regression
}

/// <summary>
/// Predict a target using a linear binary classification model trained with the <see cref="Trainers.LogisticRegressionBinaryTrainer"/> trainer.
/// Predict a target using a linear binary classification model trained with the <see cref="Trainers.LbfgsLogisticRegressionTrainer"/> trainer.
/// </summary>
/// <param name="catalog">The binary classification catalog trainer object.</param>
/// <param name="labelColumnName">The name of the label column.</param>
Expand All @@ -507,7 +507,7 @@ public static OnlineGradientDescentTrainer OnlineGradientDescent(this Regression
/// <param name="enforceNonNegativity">Enforce non-negative weights.</param>
/// <param name="l1Regularization">Weight of L1 regularization term.</param>
/// <param name="l2Regularization">Weight of L2 regularization term.</param>
/// <param name="historySize">Memory size for <see cref="Trainers.LogisticRegressionBinaryTrainer"/>. Low=faster, less accurate.</param>
/// <param name="historySize">Memory size for <see cref="Trainers.LbfgsLogisticRegressionTrainer"/>. Low=faster, less accurate.</param>
/// <param name="optimizationTolerance">Threshold for optimizer convergence.</param>
/// <example>
/// <format type="text/markdown">
Expand All @@ -516,7 +516,7 @@ public static OnlineGradientDescentTrainer OnlineGradientDescent(this Regression
/// ]]>
/// </format>
/// </example>
public static LogisticRegressionBinaryTrainer LogisticRegression(this BinaryClassificationCatalog.BinaryClassificationTrainers catalog,
public static LbfgsLogisticRegressionTrainer LogisticRegression(this BinaryClassificationCatalog.BinaryClassificationTrainers catalog,
string labelColumnName = DefaultColumnNames.Label,
string featureColumnName = DefaultColumnNames.Features,
string exampleWeightColumnName = null,
Expand All @@ -528,21 +528,21 @@ public static LogisticRegressionBinaryTrainer LogisticRegression(this BinaryClas
{
Contracts.CheckValue(catalog, nameof(catalog));
var env = CatalogUtils.GetEnvironment(catalog);
return new LogisticRegressionBinaryTrainer(env, labelColumnName, featureColumnName, exampleWeightColumnName, l1Regularization, l2Regularization, optimizationTolerance, historySize, enforceNonNegativity);
return new LbfgsLogisticRegressionTrainer(env, labelColumnName, featureColumnName, exampleWeightColumnName, l1Regularization, l2Regularization, optimizationTolerance, historySize, enforceNonNegativity);
}

/// <summary>
/// Predict a target using a linear binary classification model trained with the <see cref="Trainers.LogisticRegressionBinaryTrainer"/> trainer.
/// Predict a target using a linear binary classification model trained with the <see cref="Trainers.LbfgsLogisticRegressionTrainer"/> trainer.
/// </summary>
/// <param name="catalog">The binary classification catalog trainer object.</param>
/// <param name="options">Advanced arguments to the algorithm.</param>
public static LogisticRegressionBinaryTrainer LogisticRegression(this BinaryClassificationCatalog.BinaryClassificationTrainers catalog, LROptions options)
public static LbfgsLogisticRegressionTrainer LogisticRegression(this BinaryClassificationCatalog.BinaryClassificationTrainers catalog, LROptions options)
{
Contracts.CheckValue(catalog, nameof(catalog));
Contracts.CheckValue(options, nameof(options));

var env = CatalogUtils.GetEnvironment(catalog);
return new LogisticRegressionBinaryTrainer(env, options);
return new LbfgsLogisticRegressionTrainer(env, options);
}

/// <summary>
Expand Down
Loading