Skip to content

Conversation

@Ivanidzo4ka
Copy link
Contributor

Fix #2652

Copy link
Member

@najeeb-kazmi najeeb-kazmi left a comment

Choose a reason for hiding this comment

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

:shipit:

@Ivanidzo4ka Ivanidzo4ka requested a review from sfilipi February 26, 2019 18:58
@codecov
Copy link

codecov bot commented Feb 26, 2019

Codecov Report

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

@@            Coverage Diff            @@
##             master    #2742   +/-   ##
=========================================
  Coverage          ?   71.67%           
=========================================
  Files             ?      808           
  Lines             ?   142280           
  Branches          ?    16140           
=========================================
  Hits              ?   101978           
  Misses            ?    35862           
  Partials          ?     4440
Flag Coverage Δ
#Debug 71.67% <100%> (?)
#production 67.92% <100%> (?)
#test 85.86% <100%> (?)
Impacted Files Coverage Δ
src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs 62.06% <100%> (ø)
src/Microsoft.ML.FastTree/FastTree.cs 80.77% <100%> (ø)
...osoft.ML.Tests/TrainerEstimators/TreeEstimators.cs 98.51% <100%> (ø)

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:

FeatureColumn = "NumericFeatures",
NumTrees = 10
NumTrees = 10,
GroupIdColumn = "Group"
Copy link
Member

Choose a reason for hiding this comment

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

GroupIdColumn [](start = 20, length = 13)

I feel like this should be "GroupColumn" to fall more inline with FeatureColumn and LabelColumn.

}

protected virtual void GetDefaultParameters(IChannel ch, int numRow, bool hasCategarical, int totalCats, bool hiddenMsg=false)
protected virtual void GetDefaultParameters(IChannel ch, int numRow, bool hasCategarical, int totalCats, bool hiddenMsg = false)
Copy link
Member

Choose a reason for hiding this comment

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

hasCategarical [](start = 82, length = 14)

misspelling -- should be hasCategorical.

}

protected virtual void GetDefaultParameters(IChannel ch, int numRow, bool hasCategarical, int totalCats, bool hiddenMsg=false)
protected virtual void GetDefaultParameters(IChannel ch, int numRow, bool hasCategarical, int totalCats, bool hiddenMsg = false)
Copy link
Member

Choose a reason for hiding this comment

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

hiddenMsg [](start = 118, length = 9)

for the sake of not having short names -hiddenMessage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave that stuff to whoever would scrub lightgbm learners.

@Ivanidzo4ka Ivanidzo4ka merged commit 9b27c53 into dotnet:master Feb 26, 2019
@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.

4 participants