Skip to content

Documentation for BinaryClassification.AveragedPerceptron #2483

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
wants to merge 24 commits into from

Conversation

shmoradims
Copy link

Docs & sample for BinaryClassification.AveragedPerceptron. Related to #1209.

/// </summary>
/// <param name="catalog">The binary classification catalog trainer object.</param>
/// <param name="options">Advanced arguments to the algorithm.</param>
/// <param name="options">Advanced trainer options.</param>
public static AveragedPerceptronTrainer AveragedPerceptron(
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

this one would love sample too!
Especially how to change loss function, and change calibrator. #Resolved

Copy link
Author

@shmoradims shmoradims Feb 12, 2019

Choose a reason for hiding this comment

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

done. i don't see any public calibrator other than Platt, which is the default one. So only changed loss and a bunch of other advanced functions. Loss.Arguments() looks weird though. Users will wonder why Arguments().


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

"occupation", "relationship", "ethnicity", "native-country", "age", "education-num",
"capital-gain", "capital-loss", "hours-per-week"))
// Min-max normalized all the features
.Append(mlContext.Transforms.Normalize("Features"))
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

if this is our default pipeline for binary classification can we move it to samples utils as well? I can see how it get copied again and again in other samples. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

makes sense. I'll move it as part of data loading.


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


namespace Microsoft.ML.Samples.Dynamic.BinaryClassification
{
public static class AveragedPerceptron
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

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

Can I convince you to move this file to Dynamic/Trainers/BinaryClassification? #Resolved

Copy link
Member

Choose a reason for hiding this comment

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

+1


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

Copy link
Author

Choose a reason for hiding this comment

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

done


In reply to: 255266020 [](ancestors = 255266020,255262779)

[Argument(ArgumentType.AtMostOnce, HelpText = "Whether to shuffle for each training iteration", ShortName = "shuf")]
[TlcModule.SweepableDiscreteParamAttribute("Shuffle", new object[] { false, true })]
public bool Shuffle = true;

/// <summary>
/// Size of cache when trained in Scope.
/// </summary>
[Argument(ArgumentType.AtMostOnce, HelpText = "Size of cache when trained in Scope", ShortName = "cache")]
public int StreamingCacheSize = 1000000;

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

Can we delete this one? No one uses it.
It has no place in public API. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

deleted


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

[Argument(ArgumentType.AtMostOnce, HelpText = "Initial Weights and bias, comma-separated", ShortName = "initweights")]
[TGUI(NoSweep = true)]
public string InitialWeights;

/// <summary>
/// Initial weights scale.
/// </summary>
[Argument(ArgumentType.AtMostOnce, HelpText = "Init weights diameter", ShortName = "initwts", SortOrder = 140)]
[TGUI(Label = "Initial Weights Scale", SuggestedSweeps = "0,0.1,0.5,1")]
[TlcModule.SweepableFloatParamAttribute("InitWtsDiameter", 0.0f, 1.0f, numSteps: 5)]
public float InitWtsDiameter = 0;
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

InitWtsDiameter [](start = 21, length = 15)

you can add this one to shortname like ShortName ="initwts, initWtsDiameter" and rename it to InitialWeightsDiameter.
#Resolved

Copy link
Author

Choose a reason for hiding this comment

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

this one is easy :)


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

@codecov
Copy link

codecov bot commented Feb 8, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@a254bf5). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master    #2483   +/-   ##
=========================================
  Coverage          ?   71.19%           
=========================================
  Files             ?      787           
  Lines             ?   140996           
  Branches          ?    16111           
=========================================
  Hits              ?   100389           
  Misses            ?    36139           
  Partials          ?     4468
Flag Coverage Δ
#Debug 71.19% <0%> (?)
#production 67.55% <0%> (?)
#test 85.29% <ø> (?)

[Argument(ArgumentType.AtMostOnce, HelpText = "Number of iterations", ShortName = "iter", SortOrder = 50)]
[TGUI(Label = "Number of Iterations", Description = "Number of training iterations through data", SuggestedSweeps = "1,10,100")]
[TlcModule.SweepableLongParamAttribute("NumIterations", 1, 100, stepSize: 10, isLogScale: true)]
public int NumIterations = OnlineDefaultArgs.NumIterations;

/// <summary>
/// Initial weights and bias, comma-separated.
/// </summary>
[Argument(ArgumentType.AtMostOnce, HelpText = "Initial Weights and bias, comma-separated", ShortName = "initweights")]
[TGUI(NoSweep = true)]
public string InitialWeights;
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 8, 2019

Choose a reason for hiding this comment

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

make it internal, mark in Argument as CommandLineOnly, and create another parameter (or actually two parameters) which would accept array of floats (and single float for bias). #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

you're right, but this is a non-trivial change. our current API takes this as a string. could you please file your proposal as an issue in Project 13.


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

// Min-max normalized all the features
.Append(mlContext.Transforms.Normalize("Features"))
// Add the trainer
.Append(mlContext.BinaryClassification.Trainers.AveragedPerceptron("IsOver50K", "Features"));
Copy link
Contributor

@justinormont justinormont Feb 8, 2019

Choose a reason for hiding this comment

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

For the AP docs, please set iter=10. This is one of our more heavily benchmarked results; we decided to change the default iter to 10 a year or so back, though haven't thus far (see email: "Move AveragedPerceptron defaults to iter=10").

Could also go ahead and change the default: #2305 -- not docs/samples though will simplify the docs. #WontFix

Copy link
Author

Choose a reason for hiding this comment

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

Changing the default will change a whole bunch of test baselines. It needs a separate PR.


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

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed; many baselines will change.

.Append(mlContext.Transforms.Categorical.OneHotEncoding("occupation"))
.Append(mlContext.Transforms.Categorical.OneHotEncoding("relationship"))
.Append(mlContext.Transforms.Categorical.OneHotEncoding("ethnicity"))
.Append(mlContext.Transforms.Categorical.OneHotEncoding("native-country"))
Copy link
Contributor

@justinormont justinormont Feb 8, 2019

Choose a reason for hiding this comment

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

Can OneHotEncoding() take a set of columns to transform? It might be cleaner. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

I'd like that too. The current way to transform multiple columns in one transform is to use the overload that takes an array of ColumnInfo objects. It can't be done with just strings. I think this longer version is more readable for samples. Involving ColumnInfo is a more advanced usage and is not the main focus of this sample.

If you want to see the API that you like, please add an issue for project 13.


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

Copy link
Contributor

@justinormont justinormont Feb 12, 2019

Choose a reason for hiding this comment

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

If the ColumnInfo form of the API isn't better (or good enough for docs) we should revisit the topic. I certainly see and agree to your point that the longer form is more readable. I expect the current longer form takes N-passes of the data, which is sub-optimal. Perhaps we could make a more readable multi-input API form?

@TomFinley -- thoughts? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

to be clear, I'm not saying ColumnInfo is not good for docs. I'm saying that usage is not best for this sample which is about a binary classification task. A usage of ColumnInfo will be part of samples for OneHotEncoding.


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

@Ivanidzo4ka
Copy link
Contributor

Ivanidzo4ka commented Feb 8, 2019

// TODO: Check if it works properly if Averaged is set to false

I like this line.
It smells like confidence! #Resolved


Refers to: src/Microsoft.ML.StandardLearners/Standard/Online/AveragedLinear.cs:15 in 4222e85. [](commit_id = 4222e85, deletion_comment = False)

Copy link
Contributor

@rogancarr rogancarr left a comment

Choose a reason for hiding this comment

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

First round of comments. I like the approach here. Mostly comments about the comments.

{
public static class ConsoleUtils
{
public static void PrintBinaryClassificationMetrics(BinaryClassificationMetrics metrics)
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

How about PrintMetrics and we overload it with the appropriate metrics type? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

Good idea!


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

@@ -18,36 +18,71 @@ namespace Microsoft.ML.Trainers.Online
{
public abstract class AveragedLinearArguments : OnlineLinearArguments
{
/// <summary>
/// <a href="tmpurl_lr">Learning rate</a>
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

In the summary, just put what it is, and add any details in Remarks. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

Remarks are only visible when the user clicks on the property. Summary is readable from the owner class. I think it's more useful to have the link here, if it doesn't make the text too detailed. Then the user will have one fewer click to do. what do you think?


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

[Argument(ArgumentType.AtMostOnce, HelpText = "Learning rate", ShortName = "lr", SortOrder = 50)]
[TGUI(Label = "Learning rate", SuggestedSweeps = "0.01,0.1,0.5,1.0")]
[TlcModule.SweepableDiscreteParam("LearningRate", new object[] { 0.01, 0.1, 0.5, 1.0 })]
public float LearningRate = AveragedDefaultArgs.LearningRate;

/// <summary>
/// <see langword="true" /> to decrease the <a href="tmpurl_lr">learning rate</a> as iterations progress; otherwise, <see langword="false" />.
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

These seems more like Remarks. #Resolved

Copy link
Member

@sharwell sharwell Feb 8, 2019

Choose a reason for hiding this comment

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

A comment of this form would typically be in a <value> element. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

seems to be the correct way to do these. I changed them all to tag.


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

Copy link
Author

Choose a reason for hiding this comment

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

good point. i change them to .


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

[Argument(ArgumentType.AtMostOnce, HelpText = "Learning rate", ShortName = "lr", SortOrder = 50)]
[TGUI(Label = "Learning rate", SuggestedSweeps = "0.01,0.1,0.5,1.0")]
[TlcModule.SweepableDiscreteParam("LearningRate", new object[] { 0.01, 0.1, 0.5, 1.0 })]
public float LearningRate = AveragedDefaultArgs.LearningRate;

/// <summary>
/// <see langword="true" /> to decrease the <a href="tmpurl_lr">learning rate</a> as iterations progress; otherwise, <see langword="false" />.
/// Default is <see langword="false" />.
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

This isn't necassary; this is already given in the tooltip. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

seems to be the correct way to do these. I changed them all to tag.

also tooltip is just for intellisense. it doesn't exist in docs.com


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

Choose a reason for hiding this comment

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

It should exist in both intellisense and the docs API reference under the given class or method.

[Argument(ArgumentType.AtMostOnce, HelpText = "Decrease learning rate", ShortName = "decreaselr", SortOrder = 50)]
[TGUI(Label = "Decrease Learning Rate", Description = "Decrease learning rate as iterations progress")]
[TlcModule.SweepableDiscreteParam("DecreaseLearningRate", new object[] { false, true })]
public bool DecreaseLearningRate = AveragedDefaultArgs.DecreaseLearningRate;

/// <summary>
/// Number of examples after which weights will be reset to the current average.
/// Default is <see langword="null" />, which disables this feature.
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

No need to talk about defaults; The discussion can be moved to remarks. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

I don't think there's a standard way for this. I was basing my template on some existing dotnet docs below. We just need to be consistent.

https://docs.microsoft.com/en-us/dotnet/api/microsoft.office.interop.excel.defaultweboptions?view=excel-pia

https://docs.microsoft.com/en-us/dotnet/api/system.web.ui.updatepanel.childrenastriggers?view=netframework-4.7.2#property-value


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

Copy link
Author

Choose a reason for hiding this comment

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

seems to be the correct way to do these. I changed them all to tag.


In reply to: 255683496 [](ancestors = 255683496,255259488)


namespace Microsoft.ML.SamplesUtils
{
public static class ConsoleUtils
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

Good idea! #Resolved

// Create data processing pipeline
var pipeline =
// Convert categorical features to one-hot vectors
mlContext.Transforms.Categorical.OneHotEncoding("workclass")
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

The featurization here is a bit complex for discussing a learner. Can you please move this into the LoadAdultDataset method, with an optional parameter (default false) that applies this featurization? #Resolved

Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

Can you start on the same line as pipeline? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

i want to put // comment for each section of the pipeline. . I can't add (// Convert categorical features to one-hot vectors) if I put in the same line.


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

Copy link
Author

Choose a reason for hiding this comment

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

OneHotEncoding is the minimum we need for handling text features. we can't really use this dataset otherwise. I'm moving the pipeline to utils, but the trainer can't get an acceptable AUC by making this transform optional. agree?


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

// Min-max normalized all the features
.Append(mlContext.Transforms.Normalize("Features"))
// Add the trainer
.Append(mlContext.BinaryClassification.Trainers.AveragedPerceptron("IsOver50K", "Features"));
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

Can you do a `CopyColumns("Label", "IsOver50K") and drop the parameters, using the defaults for Label and Features? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

I don't want to give the impression that the label column must be renamed to 'Label' in order for the trainer to use it. I think we do that for a lot of other examples. It's valuable for users to see that they can use the the label column as-is.


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

{
public static void Example()
{
// In this examples we will use the adult income dataset. The goal is to predict
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

Can you move this to <Summary> and <Remarks> on Example? #Resolved

Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

IMO, let's stay away from xml documentation inside samples. I don't see any of that in .Net samples, and that is harder to read that plain text.

cc @JRAlexander


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

Copy link
Author

Choose a reason for hiding this comment

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

I checked with John. For tutorials they don't use any xml comments. Just regular // comments. we can follow the same template.

https://github.com/dotnet/samples/blob/master/machine-learning/tutorials/SentimentAnalysis/Program.cs


In reply to: 255266200 [](ancestors = 255266200,255265822)


// Create a new context for ML.NET operations. It can be used for exception tracking and logging,
// as a catalog of available operations and as the source of randomness.
// Setting the seed to a fixed number in this examples to make outputs deterministic.
Copy link
Contributor

@rogancarr rogancarr Feb 8, 2019

Choose a reason for hiding this comment

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

example #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

done


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


namespace Microsoft.ML.SamplesUtils
{
public static class ConsoleUtils
Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

ConsoleUtils [](start = 24, length = 12)

XML docs here and in the method below. #Resolved

/// Perceptron is a classification algorithm that makes its predictions based on a linear function.
/// For instance with feature values f0, f1,..., f_D-1, the prediction is given by the sign of sigma[0, D-1] (w_i * f_i), where w_0, w_1,..., w_D-1 are the weights computed by the algorithm.
///
/// Perceptron is an online algorithm, i.e., it processes the instances in the training set one at a time.
Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

Perc [](start = 12, length = 4)

will want to wrap in to create the paragraphs. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

<para> -- xml tag missing from comment on github page.


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

Copy link
Author

Choose a reason for hiding this comment

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

I'm running docfx locally and the text turns out fine as is. I think the extra /// will serve as

I'll review it in smoke-test to make sure it looks ok.


In reply to: 255269520 [](ancestors = 255269520,255266758)

/// <param name="catalog">The binary classification catalog trainer object.</param>
/// <param name="labelColumn">The name of the label column, or dependent variable.</param>
/// <param name="featureColumn">The features, or independent variables.</param>
/// <param name="lossFunction">The custom loss.</param>
/// <param name="lossFunction">The custom <a href="tmpurl_loss">loss</a>. If <see langword="null"/>, hinge loss will be used resulting in max-margin averaged perceptron.</param>
Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

tmpurl_loss [](start = 59, length = 11)

just a note to resolve when you put the actual URL #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

yes. please see #2356


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

[Argument(ArgumentType.AtMostOnce, HelpText = "Learning rate", ShortName = "lr", SortOrder = 50)]
[TGUI(Label = "Learning rate", SuggestedSweeps = "0.01,0.1,0.5,1.0")]
[TlcModule.SweepableDiscreteParam("LearningRate", new object[] { 0.01, 0.1, 0.5, 1.0 })]
public float LearningRate = AveragedDefaultArgs.LearningRate;

/// <summary>
/// <see langword="true" /> to decrease the <a href="tmpurl_lr">learning rate</a> as iterations progress; otherwise, <see langword="false" />.
Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

[](start = 52, length = 20)

i think the can live above, and here can be a cref. #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

yes. cref is better.


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

[Argument(ArgumentType.Multiple, HelpText = "Loss Function", ShortName = "loss", SortOrder = 50)]
public ISupportClassificationLossFactory LossFunction = new HingeLoss.Arguments();

/// <summary>
/// The <a href="tmpurl_calib">calibrator</a> for producing probabilities. Default is exponential (aka Platt) calibration.
Copy link
Member

@sfilipi sfilipi Feb 8, 2019

Choose a reason for hiding this comment

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

tmpurl_calib [](start = 29, length = 12)

one more #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

these are by design: #2356


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

artidoro and others added 5 commits February 8, 2019 19:03
…of ML.NET (dotnet#2470)

Adding a project for functional tests without internal access to the ML.NET Library.
…otnet#2434)

* Add analyzer for detecting BestFriend usages on public declaration

* Rewrite analyzer as Symbol analyzer

* Add test to prove IntClass.PubMember reports a warning
* removing the Microsoft.ML.Learners and the Microsoft.ML.Trainers.SymSGD namespaces.

* removing the Microsoft.ML.Internal.Internallearn.ResultProcessor and the Microsoft.ML.Trainers.FastTree.Internal namespaces.

* regenerating the catalog
codemzs and others added 2 commits February 11, 2019 12:16
* Reduce public API by making non-estimator components internal.

* Internalize forecasting code.

* Make misc stuff internal.

* build break.

* Make time series state internal.

* Hide SequenceModelerBase

* PR feedback.

* PR feedback and hide more stuff.

* fix entrypoint catalog test.

* PR feedback.

* merge conflict resolved.
* Switch OneHotEncoding parameters

* Fix table of contents

* Update for spacing
@@ -20,24 +20,40 @@ namespace Microsoft.ML.Trainers.Online

public abstract class OnlineLinearArguments : LearnerInputBaseWithLabel
{
/// <summary>
/// Number of training iterations through the data.
/// </summary>
[Argument(ArgumentType.AtMostOnce, HelpText = "Number of iterations", ShortName = "iter", SortOrder = 50)]
[TGUI(Label = "Number of Iterations", Description = "Number of training iterations through data", SuggestedSweeps = "1,10,100")]
[TlcModule.SweepableLongParamAttribute("NumIterations", 1, 100, stepSize: 10, isLogScale: true)]
public int NumIterations = OnlineDefaultArgs.NumIterations;
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Feb 11, 2019

Choose a reason for hiding this comment

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

NumIterations [](start = 19, length = 13)

Can we unify it across all codebase to NumberOfIterations? #Resolved

Copy link
Author

Choose a reason for hiding this comment

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

changed this occurrence. will update the other ones as I moved through the other learners.


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

@shmoradims
Copy link
Author

shmoradims commented Feb 11, 2019

// TODO: Check if it works properly if Averaged is set to false

somebody has to the read the paper and see what would an AveragePerceptron even mean without averaging.


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


Refers to: src/Microsoft.ML.StandardLearners/Standard/Online/AveragedLinear.cs:15 in 4222e85. [](commit_id = 4222e85, deletion_comment = False)

mareklinka and others added 13 commits February 11, 2019 12:47
* Replace ConditionalFact usages with custom facts

* Refactor ConditionalTheory usages

Add AttributeUsage attributes

* Make LessThanNetCore30OrNotNetCoreFactAttribute require an explicit skip message

* Use correct target framework moniker for netcoreapp3.0

* Remove CORECLR constant

* Remove an unused fact attribute
…pace rationalization (dotnet#2491)

* moving Microsoft.ML.TimeSeriesProcessing, and Microsoft.ML.TimeSeries to Microsoft.ML.Transforms.TimeSeries

* Microsoft.ML.Ensemble to Microsoft.ML.Trainers.Ensemble
For sample code, changed numIterations to 10 as per Justin's request
@shmoradims
Copy link
Author

Please see #2517 instead. Something got messed up during rebase. I had to recreate this PR.

@shmoradims shmoradims closed this Feb 12, 2019
@shmoradims shmoradims deleted the doc_binary_clf branch March 11, 2019 20:53
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.