Skip to content

Binary LR samples using T4 templates #3099

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 2 commits into from
Mar 28, 2019
Merged

Conversation

zeahmed
Copy link
Contributor

@zeahmed zeahmed commented Mar 26, 2019

Related to #2522. The *.cs files are auto-generated. Please review the .tt and .ttinclude files.

@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #3099 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3099      +/-   ##
==========================================
+ Coverage   72.52%   72.52%   +<.01%     
==========================================
  Files         808      808              
  Lines      144665   144665              
  Branches    16198    16198              
==========================================
+ Hits       104913   104916       +3     
+ Misses      35342    35338       -4     
- Partials     4410     4411       +1
Flag Coverage Δ
#Debug 72.52% <ø> (ø) ⬆️
#production 68.12% <ø> (ø) ⬆️
#test 88.82% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...StandardTrainers/Standard/LinearModelParameters.cs 60.05% <0%> (-0.27%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.7% <0%> (-0.21%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.1% <0%> (-0.16%) ⬇️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 74.03% <0%> (+0.33%) ⬆️
src/Microsoft.ML.Maml/MAML.cs 26.21% <0%> (+1.45%) ⬆️

@codecov
Copy link

codecov bot commented Mar 26, 2019

Codecov Report

Merging #3099 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3099      +/-   ##
==========================================
- Coverage   72.52%   72.51%   -0.01%     
==========================================
  Files         808      808              
  Lines      144665   144665              
  Branches    16198    16198              
==========================================
- Hits       104913   104907       -6     
- Misses      35342    35346       +4     
- Partials     4410     4412       +2
Flag Coverage Δ
#Debug 72.51% <ø> (-0.01%) ⬇️
#production 68.11% <ø> (-0.01%) ⬇️
#test 88.81% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...oft.ML.StandardTrainers/StandardTrainersCatalog.cs 89.07% <ø> (ø) ⬆️
src/Microsoft.ML.Transforms/Text/LdaTransform.cs 89.26% <0%> (-0.63%) ⬇️
...StandardTrainers/Standard/LinearModelParameters.cs 60.05% <0%> (-0.27%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.7% <0%> (-0.21%) ⬇️

Label = label,
// Create random features that are correlated with the label.
// For data points with false label, the feature values are slightly increased by adding a constant.
Features = Enumerable.Repeat(label, 50).Select(x => x ? randomFloat() : randomFloat() + 0.03f).ToArray()
Copy link

@shmoradims shmoradims Mar 26, 2019

Choose a reason for hiding this comment

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

0.03f [](start = 108, length = 5)

The accuracy is low. We want samples to ideally have 80%+ accuracy. Can we increase this to 0.1 and see how much accuracy improves. If so, let's keep tree samples with old 0.03f value, and increase it for less powerful trainers like LogisticRegression. #Resolved


namespace Microsoft.ML.Samples.Dynamic.Trainers.BinaryClassification
{
public static class LogisticRegressionWithOptions
Copy link

@shmoradims shmoradims Mar 26, 2019

Choose a reason for hiding this comment

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

LogisticRegressionWithOptions [](start = 24, length = 29)

please make sure the sample is added to the xml documentation for BinaryClassification.Trainers.LbfgsLogisticRegression. #Resolved

@@ -0,0 +1,29 @@
<#@ include file="BinaryClassification.ttinclude"#>
<#+
string ClassName="LogisticRegression";
Copy link

@shmoradims shmoradims Mar 26, 2019

Choose a reason for hiding this comment

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

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

let's rename class to LbfgsLogisticRegression to match the API. also plz rename the other sample to LbfgsLogisticRegressionWithOptions #Resolved

Copy link

@shmoradims shmoradims left a comment

Choose a reason for hiding this comment

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

:shipit:

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:

@zeahmed zeahmed merged commit 70c6418 into dotnet:master Mar 28, 2019
@zeahmed
Copy link
Contributor Author

zeahmed commented Mar 28, 2019

Thanks!

zeahmed added a commit to zeahmed/machinelearning that referenced this pull request Apr 8, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 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.

3 participants