Skip to content

Conversation

najeeb-kazmi
Copy link
Member

@najeeb-kazmi najeeb-kazmi commented Dec 13, 2019

Fix #4572

Incorrect trainers added to FastTreeRegression and FastTreeTweedieRegression in PR #3948

@najeeb-kazmi najeeb-kazmi requested a review from a team as a code owner December 13, 2019 01:39
@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #4573 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4573      +/-   ##
==========================================
- Coverage   75.13%    75.1%   -0.04%     
==========================================
  Files         909      909              
  Lines      160262   160262              
  Branches    17257    17257              
==========================================
- Hits       120409   120357      -52     
- Misses      35041    35087      +46     
- Partials     4812     4818       +6
Flag Coverage Δ
#Debug 75.1% <ø> (-0.04%) ⬇️
#production 70.49% <ø> (-0.04%) ⬇️
#test 90.29% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
....ML.AutoML/PipelineSuggesters/PipelineSuggester.cs 60.5% <0%> (-26.06%) ⬇️
...rosoft.ML.AutoML/Utils/SweepableParamAttributes.cs 47.89% <0%> (-10.93%) ⬇️
...rosoft.ML.AutoML/ColumnInference/TextFileSample.cs 59.6% <0%> (-2.65%) ⬇️
test/Microsoft.ML.AutoML.Tests/DatasetUtil.cs 88.69% <0%> (-2.61%) ⬇️
src/Microsoft.ML.AutoML/Sweepers/Parameters.cs 83.47% <0%> (-1.7%) ⬇️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 76.08% <0%> (-0.4%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 85.11% <0%> (-0.21%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.1% <0%> (-0.16%) ⬇️
src/Microsoft.ML.Transforms/Text/LdaTransform.cs 84.6% <0%> (+0.15%) ⬆️
src/Microsoft.ML.Maml/MAML.cs 26.21% <0%> (+1.45%) ⬆️
... and 1 more

@codemzs
Copy link
Member

codemzs commented Dec 13, 2019

        //   Label: 0.096, Prediction: 0.089

can you please update the expected output in comments .. everywhere


Refers to: docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Regression/FastTree.cs:59 in ce78d6e. [](commit_id = ce78d6e, deletion_comment = False)

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.

🕐

@sharwell
Copy link
Contributor

sharwell commented Dec 14, 2019

Are these samples building as part of the pull request builds?

Nevermind, I misread the issue here. 😄

@najeeb-kazmi
Copy link
Member Author

        //   Label: 0.096, Prediction: 0.089

Expected output needs no change. These samples used to have the correct trainers and the correct expected output. PR #3948 added the wrong trainers but did not change the expected output. So this is actually the correct expected output (verified).


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


Refers to: docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Regression/FastTree.cs:59 in ce78d6e. [](commit_id = ce78d6e, deletion_comment = False)

Copy link
Contributor

@frank-dong-ms-zz frank-dong-ms-zz left a comment

Choose a reason for hiding this comment

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

:shipit:

@dotnet dotnet locked and limited conversation to collaborators Dec 19, 2019
@dotnet dotnet unlocked this conversation Dec 19, 2019
@najeeb-kazmi najeeb-kazmi merged commit 81b04c9 into dotnet:master Dec 19, 2019
@najeeb-kazmi najeeb-kazmi deleted the fix-samples branch January 30, 2020 01:25
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 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.

FastTree regression samples use FastForest instead

5 participants