Skip to content

Add LR XML doc #3385

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

Merged
merged 9 commits into from
Apr 20, 2019
Merged

Add LR XML doc #3385

merged 9 commits into from
Apr 20, 2019

Conversation

wschin
Copy link
Member

@wschin wschin commented Apr 17, 2019

Toward #2522 following #3218.

@wschin wschin added the documentation Related to documentation of ML.NET label Apr 17, 2019
@wschin wschin self-assigned this Apr 17, 2019
@@ -27,8 +27,55 @@
namespace Microsoft.ML.Trainers
{

/// <include file='doc.xml' path='doc/members/member[@name="LBFGS"]/*' />
Copy link

@shmoradims shmoradims Apr 17, 2019

Choose a reason for hiding this comment

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

file='doc.xml' path='doc/members/member[@name="LBFGS"]/* [](start = 17, length = 56)

please delete these sections that you moved here from doc.xml #Resolved

///
/// [!include[io](~/../docs/samples/docs/api-reference/io-columns-binary-classification.md)]
///
/// ### Trainer Characteristics
Copy link
Member

@singlis singlis Apr 17, 2019

Choose a reason for hiding this comment

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

Should this be Estimator Characteristics? #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I don't see others are doing Estimator.


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

/// </summary>
/// <remarks>
/// <format type="text/markdown"><![CDATA[
/// To create this trainer, use [LbfgsLogisticRegression](xref:Microsoft.ML.StandardTrainersCatalog.LbfgsLogisticRegression(BinaryClassificationCatalog.BinaryClassificationTrainers, string, string, string, float, float, float, int, bool))
Copy link

@shmoradims shmoradims Apr 17, 2019

Choose a reason for hiding this comment

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

LbfgsLogisticRegression(BinaryClassificationCatalog.BinaryClassificationTrainers, string, string, string, float, float, float, int, bool)) [](start = 104, length = 138)

this UID won't work, you need fully qualified names. please use https://xref.docs.microsoft.com/autocomplete?text=LbfgsLogisticRegression to get the UID. It's this one:

Microsoft.ML.StandardTrainersCatalog.LbfgsLogisticRegression(Microsoft.ML.BinaryClassificationCatalog.BinaryClassificationTrainers,System.String,System.String,System.String,System.Single,System.Single,System.Single,System.Int32,System.Boolean)

please also fix for the other one.
#Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Sure.


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

@@ -39,6 +86,10 @@ public sealed partial class LbfgsLogisticRegressionBinaryTrainer : LbfgsTrainerB
internal const string Summary = "Logistic Regression is a method in statistics used to predict the probability of occurrence of an event and can "
+ "be used as a classification algorithm. The algorithm predicts the probability of occurrence of an event by fitting data to a logistical function.";

/// <summary>
/// Options for the <see cref="LbfgsLogisticRegressionBinaryTrainer"/> as used in
/// [LbfgsLogisticRegression(Options)](xref:Microsoft.ML.StandardTrainersCatalog.LbfgsLogisticRegression(BinaryClassificationCatalog.BinaryClassificationTrainers, LbfgsLogisticRegressionBinaryTrainer.Options)).
Copy link

@shmoradims shmoradims Apr 17, 2019

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

Merging #3385 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3385      +/-   ##
==========================================
+ Coverage   72.69%   72.76%   +0.06%     
==========================================
  Files         807      808       +1     
  Lines      145172   145452     +280     
  Branches    16225    16244      +19     
==========================================
+ Hits       105538   105838     +300     
+ Misses      35220    35192      -28     
- Partials     4414     4422       +8
Flag Coverage Δ
#Debug 72.76% <ø> (+0.06%) ⬆️
#production 68.27% <ø> (+0.04%) ⬆️
#test 89.04% <ø> (+0.06%) ⬆️
Impacted Files Coverage Δ
.../Standard/LogisticRegression/LogisticRegression.cs 94.95% <ø> (ø) ⬆️
...oft.ML.StandardTrainers/StandardTrainersCatalog.cs 92.34% <ø> (ø) ⬆️
src/Microsoft.ML.Data/Transforms/KeyToValue.cs 79.16% <0%> (-0.65%) ⬇️
src/Microsoft.ML.Transforms/Text/LdaTransform.cs 89.26% <0%> (-0.63%) ⬇️
...ML.Transforms/MutualInformationFeatureSelection.cs 78.58% <0%> (-0.33%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.1% <0%> (-0.16%) ⬇️
...ft.ML.Data/Evaluators/BinaryClassifierEvaluator.cs 77.18% <0%> (-0.01%) ⬇️
...ML.Data/Transforms/ColumnConcatenatingEstimator.cs 80.3% <0%> (ø) ⬆️
...soft.ML.Transforms/Text/WordEmbeddingsExtractor.cs 87.52% <0%> (ø) ⬆️
...luators/Metrics/MulticlassClassificationMetrics.cs 100% <0%> (ø) ⬆️
... and 57 more

@@ -518,11 +518,11 @@ public IClassificationLoss CreateComponent(IHostEnvironment env)
}

/// <summary>
/// Predict a target using a linear binary classification model trained with the <see cref="Trainers.LbfgsLogisticRegressionBinaryTrainer"/> trainer.
/// Create an <see cref="Trainers.LbfgsLogisticRegressionBinaryTrainer"/>, which predicts a target using a linear binary classification model trained over boolean label data.
Copy link
Member

@codemzs codemzs Apr 19, 2019

Choose a reason for hiding this comment

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

an [](start = 20, length = 2)

Can you please check with Shahab/@natke if we need "an"? In my PRs it was just Create
#Resolved

Copy link

@shmoradims shmoradims Apr 19, 2019

Choose a reason for hiding this comment

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

"Create a" or just "Create" #Resolved

Copy link
Member

@codemzs codemzs left a comment

Choose a reason for hiding this comment

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

:shipit:

/// An accurate model with extreme coefficient values would be penalized more, but a less accurate model with more conservative values would be penalized less.
///
/// This learner supports [elastic net regularization](https://en.wikipedia.org/wiki/Elastic_net_regularization): a linear combination of L1-norm (LASSO), $|| \boldsymbol{w} ||_1$, and L2-norm (ridge), $|| \boldsymbol{w} ||_2^2$ regularizations.
/// L1-nrom and L2-norm regularizations have different effects and uses that are complementary in certain respects.
Copy link
Member

@singlis singlis Apr 20, 2019

Choose a reason for hiding this comment

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

r [](start = 12, length = 1)

L1-norm #Resolved

/// This learner supports [elastic net regularization](https://en.wikipedia.org/wiki/Elastic_net_regularization): a linear combination of L1-norm (LASSO), $|| \boldsymbol{w} ||_1$, and L2-norm (ridge), $|| \boldsymbol{w} ||_2^2$ regularizations.
/// L1-nrom and L2-norm regularizations have different effects and uses that are complementary in certain respects.
/// Using L1-norm can increase sparsity of the trained $\boldsymbol{w}$.
/// When working with high-dimensional data, it shrinks small weights of irrevalent features to 0 and therefore no reource will be spent on those bad features when making prediction.
Copy link
Member

@singlis singlis Apr 20, 2019

Choose a reason for hiding this comment

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

irrevalent [](start = 77, length = 10)

irrelavent #Resolved

Copy link
Member

@singlis singlis left a comment

Choose a reason for hiding this comment

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

:shipit:

@wschin wschin merged commit 6cb5a37 into dotnet:master Apr 20, 2019
@wschin wschin deleted the lr-doc branch April 20, 2019 07:22
@ghost ghost locked as resolved and limited conversation to collaborators Mar 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Related to documentation of ML.NET
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants