More pigstensions #1084

sfilipi · 2018-09-27T23:57:27Z

ongoing work to address #754.
This PR adds the xtensions for AP, OGD, LR, Multi-LR, and Poisson Regression.

adding pigstensions for lr, multilr, possion

Removing the AP xtension that produces the probability column.

sfilipi · 2018-09-27T23:59:48Z

src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs

+        /// the linear model that was trained.  Note that this action cannot change the result in any way; it is only a way for the caller to
+        /// be informed about what was learnt.</param>
+        /// <returns>The predicted output.</returns>
+        public static (Scalar<float> score, Scalar<float> probability, Scalar<bool> predictedLabel) LogisticRegression(this BinaryClassificationContext.BinaryClassificationTrainers ctx,


LogisticRegression [](start = 100, length = 18)

rename to LogisticRegressionBinaryClassifier #Resolved

sfilipi · 2018-09-28T00:01:07Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineLearnerStatic.cs

+                    }
+                    return trainer;
+                    */
+


remove #Resolved

sfilipi · 2018-09-28T00:01:21Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineLearnerStatic.cs

+        /// <summary>
+        /// Predict a target using a linear binary classification model trained with the SDCA trainer, and a custom loss.
+        /// Note that because we cannot be sure that all loss functions will produce naturally calibrated outputs, setting
+        /// a custom loss function will not produce a calibrated probability column.


remove #Resolved

sfilipi · 2018-09-28T00:02:02Z

test/Microsoft.ML.StaticPipelineTesting/Training.cs

-                .Append(r => (r.label, preds: ctx.Trainers.Sdca(r.label, r.features,
-                maxIterations: 2,
-                loss: loss, onFit: p => pred = p)));
+                .Append(r => (r.label, preds: ctx.Trainers.AveragedPerceptron(loss, r.label, r.features,


AveragedPerceptron [](start = 59, length = 18)

revert #Resolved

sfilipi · 2018-09-28T00:02:58Z

src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs

+using Microsoft.ML.StaticPipe;
+using Microsoft.ML.StaticPipe.Runtime;
+
+namespace Microsoft.ML.Trainers


Microsoft.ML.Trainers [](start = 10, length = 21)

change namespace #Closed

discussion on the other comment.

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

sfilipi · 2018-09-28T00:03:11Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineLearnerStatic.cs

+using Microsoft.ML.StaticPipe.Runtime;
+using System;
+
+namespace Microsoft.ML.Trainers


Microsoft.ML.Trainers [](start = 10, length = 21)

change namespace #Resolved

Actually moving to ML.StaticPipe namespace won't fix it by itself. All the contexts are in Microsoft.ML.

@TomFinley move the contexts in StaticPipe as well?

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

No. If the user opts into the static pipe, they will see the trainers. It's good enough for me

In reply to: 221299214 [](ancestors = 221299214,221111449)

The context is in Microsoft.ML, i don't see anything import staticPipe.

In reply to: 221307154 [](ancestors = 221307154,221299214,221111449)

Ivanidzo4ka · 2018-09-28T00:15:21Z

test/Microsoft.ML.Tests/TrainerEstimators/OnlineLinearTests.cs

@@ -25,7 +25,7 @@ public void OnlineLinearWorkout()

            var trainData = pipe.Fit(data).Transform(data).AsDynamic;

-            IEstimator<ITransformer> est = new OnlineGradientDescentTrainer(Env, new OnlineGradientDescentTrainer.Arguments());
+            IEstimator<ITransformer> est = new OnlineGradientDescentTrainer(Env, "Label", "Features");


"Label" [](start = 81, length = 7)

Any chance I can convince you to use DefaultColumnNames.Label? #Pending

I can change, it; it won't be any better than it is.
This is "Label", because in like 20 we defined r.Label.

There is no way to type the the member name to be the same as a particular string.
(might be the only thing i miss from javascript :P)

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

Ivanidzo4ka · 2018-09-28T00:17:42Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineGradientDescent.cs

+        /// <param name="weightsColumn">The name of the weights column.</param>
+        /// <param name="lossFunction">The custom loss functions. Defaults to <see cref="SquaredLoss"/> if not provided.</param>
+        public OnlineGradientDescentTrainer(IHostEnvironment env,
+            string labelColumn,


labelColumn [](start = 19, length = 11)

any reason not to use default values here? #Pending

i think we have marked the label and feature columns as required everywhere.

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

I think we were not consistent.

In reply to: 221298419 [](ancestors = 221298419,221113276)

@Zruty0 which one are you referring too? Change anything?

In reply to: 221307649 [](ancestors = 221307649,221298419,221113276)

Ivanidzo4ka · 2018-09-28T00:29:17Z

src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsPredictorBase.cs

-            L1Weight = Args.L1Weight;
-            OptTol = Args.OptTol;
-            MemorySize = Args.MemorySize;
+            Contracts.CheckParam(l2Weight >= 0, nameof(l2Weight), "Must be non-negative, if provided.");


Contracts [](start = 12, length = 9)

why not Host? #Resolved

thanks!

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

Ivanidzo4ka · 2018-09-28T00:33:05Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineLearnerStatic.cs

+                Action<LinearBinaryPredictor> onFit = null
+            )
+        {
+            Contracts.CheckValue(label, nameof(label));


Contracts [](start = 12, length = 9)

technically you have ctx.Environment #Pending

ctx is just the trainers, not the context. I don't have the env.

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

Oh, you do. You need to call StaticPipeUtils.GetEnvironment to get it

In reply to: 221295921 [](ancestors = 221295921,221115177)

That requires a SchemaBearing, as argument. Doesn't seem worth to construct one to just check on the params..

In reply to: 221307369 [](ancestors = 221307369,221295921,221115177)

Zruty0 · 2018-09-28T16:18:40Z

src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs

+
+    }
+
+    internal sealed class LbfgsStaticsUtils{


Statics [](start = 31, length = 7)

static ? #Resolved

also the class itself should be static

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

Zruty0

artidoro · 2018-09-28T17:45:39Z

src/Microsoft.ML.StandardLearners/Standard/LogisticRegression/LbfgsStatics.cs

+        /// <param name="features">The features, or independent variables.</param>
+        /// <param name="weights">The optional example weights.</param>
+        /// <param name="enoforceNoNegativity">Enforce non-negative weights.</param>
+        /// <param name="l1Weight">Weight of L1 regularizer term.</param>


Maybe it is more standard to say regularization, instead of regularizer. #Resolved

more checks on user params

artidoro · 2018-09-28T17:53:52Z

using Float = System.Single;

If you have time, would be a good idea to clean this up. #Resolved

Refers to: src/Microsoft.ML.StandardLearners/Standard/Online/OnlineGradientDescent.cs:5 in f414717. [](commit_id = f414717, deletion_comment = False)

artidoro · 2018-09-28T17:54:22Z

src/Microsoft.ML.StandardLearners/Standard/Online/OnlineGradientDescent.cs

+            string labelColumn,
+            string featureColumn,
+            float learningRate = Arguments.OgdDefaultArgs.LearningRate,
+            bool decreaseLearningRate =Arguments.OgdDefaultArgs.DecreaseLearningRate,


Add a space after equal. #Resolved

artidoro · 2018-09-28T17:58:40Z

src/Microsoft.ML.StandardLearners/Standard/PoissonRegression/PoissonRegression.cs

@@ -46,10 +46,22 @@ public sealed class Arguments : ArgumentsBase
        /// <param name="labelColumn">The name of the label column.</param>
        /// <param name="featureColumn">The name of the feature column.</param>
        /// <param name="weightColumn">The name for the example weight column.</param>
+        /// <param name="enforceNoNegativity">Enforce non-negative weights.</param>
+        /// <param name="l1Weight">Weight of L1 regularizer term.</param>


regularizer [](start = 48, length = 11)

regularizer pops up in many files, let me see if I can help you find them.

artidoro

off by one fix .

sfilipi added 3 commits September 27, 2018 13:56

AP xtensions

652472c

lbfgs derived classes take more arguments in their public ctors

5b5927c

adding pigstensions for lr, multilr, possion

Ogd static xtensions.

60df43c

Removing the AP xtension that produces the probability column.

sfilipi commented Sep 27, 2018

View reviewed changes

sfilipi commented Sep 28, 2018

View reviewed changes

sfilipi requested review from Zruty0, artidoro, GalOshri, TomFinley and Ivanidzo4ka September 28, 2018 00:03

sfilipi self-assigned this Sep 28, 2018

sfilipi added the API Issues pertaining the friendly API label Sep 28, 2018

Ivanidzo4ka reviewed Sep 28, 2018

View reviewed changes

comment, and fixing the check conditions.

87530b9

Zruty0 reviewed Sep 28, 2018

View reviewed changes

Zruty0 approved these changes Sep 28, 2018

View reviewed changes

sfilipi added 2 commits September 28, 2018 10:26

namespace change for pigstensions

2b58625

merging from master

f414717

artidoro reviewed Sep 28, 2018

View reviewed changes

sfilipi added 2 commits September 28, 2018 10:49

namespace change,

03c1cdc

more checks on user params

casing and help sentences nit

dabf591

artidoro reviewed Sep 28, 2018

View reviewed changes

sfilipi added 2 commits September 28, 2018 10:55

cleaning up System.Single

2f42b80

ctrl+k+d

e01ff3f

artidoro reviewed Sep 28, 2018

View reviewed changes

artidoro approved these changes Sep 28, 2018

View reviewed changes

restoring the change on the solution.

35a065f

off by one fix .

sfilipi merged commit 0b9ca00 into dotnet:master Sep 28, 2018

sfilipi deleted the morePigstensions branch September 28, 2018 21:02

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More pigstensions #1084

More pigstensions #1084

sfilipi commented Sep 27, 2018 •

edited

Loading

sfilipi Sep 27, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018

Zruty0 Sep 28, 2018

sfilipi Sep 28, 2018 •

edited

Loading

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

sfilipi Sep 28, 2018

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

sfilipi Sep 28, 2018

Zruty0 Sep 28, 2018

sfilipi Sep 28, 2018

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

sfilipi Sep 28, 2018

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

sfilipi Sep 28, 2018

Zruty0 Sep 28, 2018

sfilipi Sep 28, 2018

Zruty0 Sep 28, 2018 •

edited by sfilipi

Loading

Zruty0 Sep 28, 2018

Zruty0 left a comment

artidoro Sep 28, 2018 •

edited by sfilipi

Loading

artidoro commented Sep 28, 2018 •

edited by sfilipi

Loading

artidoro Sep 28, 2018 •

edited by sfilipi

Loading

artidoro Sep 28, 2018

artidoro left a comment

More pigstensions #1084

More pigstensions #1084

Conversation

sfilipi commented Sep 27, 2018 • edited Loading

sfilipi Sep 27, 2018 • edited Loading

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfilipi Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Sep 28, 2018 • edited by sfilipi Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Sep 28, 2018 • edited by sfilipi 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

Ivanidzo4ka Sep 28, 2018 • edited by sfilipi Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Sep 28, 2018 • edited by sfilipi 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

Zruty0 Sep 28, 2018 • edited by sfilipi Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zruty0 left a comment

Choose a reason for hiding this comment

artidoro Sep 28, 2018 • edited by sfilipi Loading

Choose a reason for hiding this comment

artidoro commented Sep 28, 2018 • edited by sfilipi Loading

artidoro Sep 28, 2018 • edited by sfilipi Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artidoro left a comment

Choose a reason for hiding this comment

sfilipi commented Sep 27, 2018 •

edited

Loading

sfilipi Sep 27, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

sfilipi Sep 28, 2018 •

edited

Loading

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

Ivanidzo4ka Sep 28, 2018 •

edited by sfilipi

Loading

Zruty0 Sep 28, 2018 •

edited by sfilipi

Loading

artidoro Sep 28, 2018 •

edited by sfilipi

Loading

artidoro commented Sep 28, 2018 •

edited by sfilipi

Loading

artidoro Sep 28, 2018 •

edited by sfilipi

Loading