Skip to content

Scrub changes for LightGBM #2808

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.

Sign up for GitHub

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
singlis merged 9 commits into from
Mar 5, 2019
Merged

Scrub changes for LightGBM #2808

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8dd53e1
Api clean up for LightGBM. The cleanup includes:
singlis Mar 1, 2019
23812b5
Merge remote-tracking branch 'upstream/master' into singlis/scrub-lig…
singlis Mar 1, 2019
ce21121
- Updating from feedback
singlis Mar 2, 2019
b958c37
Merge remote-tracking branch 'origin/master' into singlis/scrub-lightgbm
singlis Mar 2, 2019
90e3ee2
- Minor updates
singlis Mar 2, 2019
d32e44e
Merge remote-tracking branch 'origin/master' into singlis/scrub-lightgbm
singlis Mar 4, 2019
27172f1
- Updating based upon feedback
singlis Mar 5, 2019
a355b31
Merge remote-tracking branch 'origin/master' into singlis/scrub-lightgbm
singlis Mar 5, 2019
4a1fbb0
Merge remote-tracking branch 'origin/master' into singlis/scrub-lightgbm
singlis Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Ranking/LightGbm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ public static void Example()

// Create the Estimator pipeline. For simplicity, we will train a small tree with 4 leaves and 2 boosting iterations.
var pipeline = mlContext.Ranking.Trainers.LightGbm(
numLeaves: 4,
minDataPerLeaf: 10,
leafCount: 4,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
leafCount: 4,
numberOfLeaves: 4,

as suggested in #2613. #Resolved

minimumDataPerLeaf: 10,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
minimumDataPerLeaf: 10,
MinimumExampleCountPerLeaf: 10,
``` #Resolved

@yaeldekel yaeldekel Mar 1, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should the arguments have the same name as FastTreeArguments?
In FastTree these arguments are called numTrees and minDatapointsInLeaves.

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

@yaeldekel yaeldekel Mar 1, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So according to issue #2613 numTrees should change to numberOfTrees (or numberOfIterations, as long as we are consistent with FastTree).

In reply to: 261698972 [](ancestors = 261698972,261456738)

@singlis singlis Mar 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My preference is that they should follow what is in LightGBM, but in a more verbose form to align with what we have in C#. For example, num_iterations == NumberOfIterations, min_data_in_leaf == minimumDataInLeaf. This way they bear some resemblance of the LightGBM arguments if a user looks it up.

In reply to: 261699860 [](ancestors = 261699860,261698972,261456738)

@singlis singlis Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Resolved to we use our own parameter names and use a mapping of names -> light gbm arguments. Parameter names that are the same between FastTree and LightGBM will use the same name.

In reply to: 261722064 [](ancestors = 261722064,261699860,261698972,261456738)

@singlis singlis Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@yaeld@microsoft.com Wei-Sheng has a PR for fast tree that is updating the parameters to match with the changes that we are doing here.

In reply to: 261698972 [](ancestors = 261698972,261456738)

learningRate: 0.1,
numBoostRound: 2);
numberOfIterations: 2);

// Fit this Pipeline to the Training Data.
var model = pipeline.Fit(split.TrainSet);
Expand Down
6 changes: 3 additions & 3 deletions docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Ranking/LightGbmWithOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ public static void Example()
var pipeline = mlContext.Ranking.Trainers.LightGbm(
new Options
{
NumLeaves = 4,
MinDataPerLeaf = 10,
NumberOfLeaves = 4,
MinimumDataPerLeaf = 10,
LearningRate = 0.1,
NumBoostRound = 2,
NumberOfIterations = 2,
Booster = new TreeBooster.Options
{
FeatureFraction = 0.9
Expand Down
4 changes: 2 additions & 2 deletions docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Regression/LightGbm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public static void Example()
var pipeline = mlContext.Transforms.Concatenate("Features", featureNames)
.Append(mlContext.Regression.Trainers.LightGbm(
labelColumnName: labelName,
numLeaves: 4,
minDataPerLeaf: 6,
leafCount: 4,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
leafCount: 4,
numberOfLeaves: 4,

as suggested in #2613. I can follow you but we need to change other places where #2613 is already applied. #Resolved

minimumDataPerLeaf: 6,
learningRate: 0.001));

// Fit this pipeline to the training data.
Expand Down
4 changes: 2 additions & 2 deletions docs/samples/Microsoft.ML.Samples/Dynamic/Trainers/Regression/LightGbmWithOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public static void Example()
.Append(mlContext.Regression.Trainers.LightGbm(new Options
{
LabelColumnName = labelName,
NumLeaves = 4,
MinDataPerLeaf = 6,
NumberOfLeaves = 4,
MinimumDataPerLeaf = 6,
LearningRate = 0.001,
Booster = new GossBooster.Options
{
Expand Down
4 changes: 2 additions & 2 deletions docs/samples/Microsoft.ML.Samples/Static/LightGBMRegression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public static void LightGbmRegression()
.Append(r => (r.label, score: mlContext.Regression.Trainers.LightGbm(
r.label,
r.features,
numLeaves: 4,
minDataPerLeaf: 6,
numberOfLeaves: 4,
minimumDataPerLeaf: 6,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
minimumDataPerLeaf: 6,
minimumExampleCountPerLeaf: 6,
``` #Resolved

learningRate: 0.001,
onFit: p => pred = p)
)
Expand Down
65 changes: 35 additions & 30 deletions src/Microsoft.ML.LightGBM.StaticPipe/LightGbmStaticExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ public static class LightGbmStaticExtensions
/// <param name="label">The label column.</param>
/// <param name="features">The features column.</param>
/// <param name="weights">The weights column.</param>
/// <param name="numLeaves">The number of leaves to use.</param>
/// <param name="numBoostRound">Number of iterations.</param>
/// <param name="minDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="numberOfLeaves">The number of leaves to use.</param>
/// <param name="minimumDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// <param name="minimumDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="minimumExampleCountPerLeaf">The minimal number of data points allowed in a leaf of the tree, out of the subsampled data.</param>
``` #Resolved

/// <param name="learningRate">The learning rate.</param>
/// <param name="numberOfIterations">Number of iterations.</param>
/// <param name="onFit">A delegate that is called every time the
/// <see cref="Estimator{TInShape, TOutShape, TTransformer}.Fit(DataView{TInShape})"/> method is called on the
/// <see cref="Estimator{TInShape, TOutShape, TTransformer}"/> instance created out of this. This delegate will receive
Expand All @@ -39,19 +39,19 @@ public static class LightGbmStaticExtensions
/// </example>
public static Scalar<float> LightGbm(this RegressionCatalog.RegressionTrainers catalog,
Scalar<float> label, Vector<float> features, Scalar<float> weights = null,
int? numLeaves = null,
int? minDataPerLeaf = null,
int? numberOfLeaves = null,
int? minimumDataPerLeaf = null,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int? minimumDataPerLeaf = null,
int? minimumExampleCountPerLeaf = null,
``` #Resolved

double? learningRate = null,
int numBoostRound = Options.Defaults.NumBoostRound,
int numberOfIterations = Options.Defaults.NumberOfIterations,
Action<LightGbmRegressionModelParameters> onFit = null)
{
CheckUserValues(label, features, weights, numLeaves, minDataPerLeaf, learningRate, numBoostRound, onFit);
CheckUserValues(label, features, weights, numberOfLeaves, minimumDataPerLeaf, learningRate, numberOfIterations, onFit);

var rec = new TrainerEstimatorReconciler.Regression(
(env, labelName, featuresName, weightsName) =>
{
var trainer = new LightGbmRegressorTrainer(env, labelName, featuresName, weightsName, numLeaves,
minDataPerLeaf, learningRate, numBoostRound);
var trainer = new LightGbmRegressorTrainer(env, labelName, featuresName, weightsName, numberOfLeaves,
minimumDataPerLeaf, learningRate, numberOfIterations);
if (onFit != null)
return trainer.WithOnFitDelegate(trans => onFit(trans.Model));
return trainer;
Expand Down Expand Up @@ -122,11 +122,13 @@ public static Scalar<float> LightGbm(this RegressionCatalog.RegressionTrainers c
/// ]]></format>
/// </example>
public static (Scalar<float> score, Scalar<float> probability, Scalar<bool> predictedLabel) LightGbm(this BinaryClassificationCatalog.BinaryClassificationTrainers catalog,
Scalar<bool> label, Vector<float> features, Scalar<float> weights = null,
Scalar<bool> label,
Vector<float> features,
Scalar<float> weights = null,
int? numLeaves = null,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int? numLeaves = null,
int? numberOfLeaves = null,
``` #Resolved

int? minDataPerLeaf = null,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int? minDataPerLeaf = null,
int? minimumExampleCountPerLeaf = null,
``` #Resolved

double? learningRate = null,
int numBoostRound = Options.Defaults.NumBoostRound,
int numBoostRound = Options.Defaults.NumberOfIterations,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int numBoostRound = Options.Defaults.NumberOfIterations,
int numberOfBoostRound = Options.Defaults.NumberOfIterations,
``` #Resolved

@singlis singlis Mar 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

resolved to numberOfIterations

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

Action<CalibratedModelParametersBase<LightGbmBinaryModelParameters, PlattCalibrator>> onFit = null)
{
CheckUserValues(label, features, weights, numLeaves, minDataPerLeaf, learningRate, numBoostRound, onFit);
Expand Down Expand Up @@ -194,9 +196,9 @@ public static (Scalar<float> score, Scalar<float> probability, Scalar<bool> pred
/// <param name="features">The features column.</param>
/// <param name="groupId">The groupId column.</param>
/// <param name="weights">The weights column.</param>
/// <param name="numLeaves">The number of leaves to use.</param>
/// <param name="numBoostRound">Number of iterations.</param>
/// <param name="minDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="numberOfLeaves">The number of leaves to use.</param>
/// <param name="numberOfIterations">Number of iterations.</param>
/// <param name="minimumDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="learningRate">The learning rate.</param>
/// <param name="onFit">A delegate that is called every time the
/// <see cref="Estimator{TInShape, TOutShape, TTransformer}.Fit(DataView{TInShape})"/> method is called on the
Expand All @@ -206,21 +208,24 @@ public static (Scalar<float> score, Scalar<float> probability, Scalar<bool> pred
/// <returns>The set of output columns including in order the predicted binary classification score (which will range
/// from negative to positive infinity), the calibrated prediction (from 0 to 1), and the predicted label.</returns>
public static Scalar<float> LightGbm<TVal>(this RankingCatalog.RankingTrainers catalog,
Scalar<float> label, Vector<float> features, Key<uint, TVal> groupId, Scalar<float> weights = null,
int? numLeaves = null,
int? minDataPerLeaf = null,
Scalar<float> label,
Vector<float> features,
Key<uint, TVal> groupId,
Scalar<float> weights = null,
int? numberOfLeaves = null,
int? minimumDataPerLeaf = null,

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
int? minimumDataPerLeaf = null,
int? minimumExampleCountPerLeaf = null,
``` #Resolved

double? learningRate = null,
int numBoostRound = Options.Defaults.NumBoostRound,
int numberOfIterations = Options.Defaults.NumberOfIterations,
Action<LightGbmRankingModelParameters> onFit = null)
{
CheckUserValues(label, features, weights, numLeaves, minDataPerLeaf, learningRate, numBoostRound, onFit);
CheckUserValues(label, features, weights, numberOfLeaves, minimumDataPerLeaf, learningRate, numberOfIterations, onFit);
Contracts.CheckValue(groupId, nameof(groupId));

var rec = new TrainerEstimatorReconciler.Ranker<TVal>(
(env, labelName, featuresName, groupIdName, weightsName) =>
{
var trainer = new LightGbmRankingTrainer(env, labelName, featuresName, groupIdName, weightsName, numLeaves,
minDataPerLeaf, learningRate, numBoostRound);
var trainer = new LightGbmRankingTrainer(env, labelName, featuresName, groupIdName, weightsName, numberOfLeaves,
minimumDataPerLeaf, learningRate, numberOfIterations);

if (onFit != null)
return trainer.WithOnFitDelegate(trans => onFit(trans.Model));
Expand Down Expand Up @@ -279,10 +284,10 @@ public static Scalar<float> LightGbm<TVal>(this RankingCatalog.RankingTrainers c
/// <param name="label">The label, or dependent variable.</param>
/// <param name="features">The features, or independent variables.</param>
/// <param name="weights">The weights column.</param>
/// <param name="numLeaves">The number of leaves to use.</param>
/// <param name="numBoostRound">Number of iterations.</param>
/// <param name="minDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="numberOfLeaves">The number of leaves to use.</param>
/// <param name="minimumDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>

@wschin wschin Mar 1, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// <param name="minimumDataPerLeaf">The minimal number of documents allowed in a leaf of the tree, out of the subsampled data.</param>
/// <param name="minimumDataPerLeaf">The minimal number of data points allowed in a leaf of the tree, out of the subsampled data.</param>
``` #Resolved

@singlis singlis Mar 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am using minimumExampleCountPerLeaf based on the previous comments.

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

@singlis singlis Mar 1, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also replaced all comments with what you provided above.

In reply to: 261777274 [](ancestors = 261777274,261458306)

/// <param name="learningRate">The learning rate.</param>
/// <param name="numberOfIterations">Number of iterations.</param>
/// <param name="onFit">A delegate that is called every time the
/// <see cref="Estimator{TInShape, TOutShape, TTransformer}.Fit(DataView{TInShape})"/> method is called on the
/// <see cref="Estimator{TInShape, TOutShape, TTransformer}"/> instance created out of this. This delegate will receive
Expand All @@ -301,19 +306,19 @@ public static (Vector<float> score, Key<uint, TVal> predictedLabel)
Key<uint, TVal> label,
Vector<float> features,
Scalar<float> weights = null,
int? numLeaves = null,
int? minDataPerLeaf = null,
int? numberOfLeaves = null,
int? minimumDataPerLeaf = null,

@wschin wschin Mar 2, 2019 •
edited by singlis
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minimumDataPerLeaf [](start = 17, length = 18)

minimumExampleCountPerLeaf #Resolved

double? learningRate = null,
int numBoostRound = Options.Defaults.NumBoostRound,
int numberOfIterations = Options.Defaults.NumberOfIterations,
Action<OvaModelParameters> onFit = null)
{
CheckUserValues(label, features, weights, numLeaves, minDataPerLeaf, learningRate, numBoostRound, onFit);
CheckUserValues(label, features, weights, numberOfLeaves, minimumDataPerLeaf, learningRate, numberOfIterations, onFit);

var rec = new TrainerEstimatorReconciler.MulticlassClassifier<TVal>(
(env, labelName, featuresName, weightsName) =>
{
var trainer = new LightGbmMulticlassTrainer(env, labelName, featuresName, weightsName, numLeaves,
minDataPerLeaf, learningRate, numBoostRound);
var trainer = new LightGbmMulticlassTrainer(env, labelName, featuresName, weightsName, numberOfLeaves,
minimumDataPerLeaf, learningRate, numberOfIterations);

if (onFit != null)
return trainer.WithOnFitDelegate(trans => onFit(trans.Model));
Expand Down
Loading