Monotone constraint support for LightGBM #2330

singlis · 2019-01-30T19:37:04Z

This add the ability to set the monotone_constraints argument for
LightGBM. This is done through the LightGBM Options class via the
MonotoneConstraints member. To handle the monotone constraints, this
adds the ability to specify whether to use a positive constraint or a
negative constraint along with a range. Multiple ranges can be
specified.

This checkin also includes tests for the parsing of the ranges to
validate the expected value that will be passed to LightGBM.

This fixes #1651.

LightGBM. This is done through the LightGBM Options class via the MonotoneConstraints member. To handle the monotone constraints, this adds the ability to specify whether to use a positive constraint or a negative constraint along with a range. Multiple ranges can be specified. This checkin also includes tests for the parsing of the ranges to validate the expected value that will be passed to LightGBM. This fixes dotnet#1651.

wschin · 2019-01-30T19:47:32Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

+
+            var transformedDataView = pipe.Fit(dataView).Transform(dataView);
+            var model = trainer.Train(transformedDataView, transformedDataView);
+            Done();


Could you check if prediction values are really monotone? For example, feeding 10 examples such as [0, 0, 1, 0], [0, 0, 2, 0], [0, 0, 3, 0], ... into the trained model and then examine their prediction values. #Resolved

justinormont · 2019-01-30T21:17:43Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+                var subArguments = argument.Split(':');
+                if (subArguments.Length != 2)
+                    // Invalid argument, skip to the next argument
+                    continue;


I don't think it's reasonable to force a user to know their slot numbers. The exact contents of the slots are generally a blackbox to ML.NET users (exception is for a purely numeric dataset without featurization which alters the slots).

Perhaps we could add support for just a plain "pos", "neg" keyword which then applies to all slots. #Resolved

I agree - so I added this feature along with tests.

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

@singlis: Is the change visible in the PR? Or is it yet to be committed? #Resolved

I just published this.

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

Thanks! #Resolved

justinormont · 2019-01-31T05:51:24Z

@Ivanidzo4ka brought up in #1992 (comment):

I notice some work by @singlis regarding LightGBM and bringing monotonic support. Do you have time to look on random forest support?

Would the Random Forest support be a good addition to this PR? #Resolved

…notone

Ivanidzo4ka · 2019-02-01T19:03:08Z

src/Microsoft.ML.LightGBM/LightGbmArguments.cs

+            "specified followed by a range. For example, pos:0-2 neg:3,5 will apply a positive constraint to the " +
+            "first three features and a negative constraint to the 4th and 6th feature. If feature index is not specified, " +
+            "then no constraint will be applied.")]
+        public string[] MonotoneConstraints;


public string[] MonotoneConstraints; [](start = 8, length = 36)

A) please unblock and run test RegenerateEntryPointCatalog on your local machine and push changes in core_ep-list.tsv.
B) How it would handle null value? I don't see test covering that.
C) How it would handle gibberish values? Like Bubba, TabbyCat ? What kind of exception it throw, was that exception be enough to understand something is wrong with this parameter? #Closed

Ivanidzo4ka · 2019-02-01T19:05:12Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

@@ -61,6 +61,124 @@ public void LightGBMBinaryEstimator()
            Done();
        }

+        [ConditionalFact(typeof(Environment), nameof(Environment.Is64BitProcess))] // LightGBM is 64-bit only
+        public void LightGBMBinaryEstimatorMonotoneThroughOptions()


LightGBMBinaryEstimatorMonotoneThroughOptions [](start = 20, length = 45)

Can we also have baseline tests?
we have tests like this: RegressorLightGBMTest
var regPredictors = new[] { TestLearners.LightGBMReg };
you can define new learner like
public static PredictorAndArgs LightGBMRegWithMonotone = new PredictorAndArgs { Trainer = new SubComponent("LightGBMR", "nt=1 iter=50 v=+ booster=gbdt{l1=0.2 l2=0.2} lr=0.2 mil=10 nl=20 MonotoneConstraints=pos:1-3 MonotoneConstraints=neg:4"), Tag = "LightGBMReg", BaselineProgress = true, };
and add it to regPredictors.
#Closed

same for all types of trainers, but it shouldn't be hard I believe. #Closed

added for Regression, MC and Classification

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

Ivanidzo4ka · 2019-02-01T19:06:08Z

src/Microsoft.ML.LightGBM/LightGbmArguments.cs

@@ -371,6 +371,14 @@ public enum EvalMetricType
        [TlcModule.SweepableDiscreteParam("CatL2", new object[] { 0.1, 0.5, 1, 5, 10 })]
        public double CatL2 = 10;

+        [Argument(ArgumentType.Multiple,


Argument [](start = 9, length = 8)

please add ShortName, if someone would ever need to use that in command line, he would curse you for typing this words again and again. #Closed

Ivanidzo4ka · 2019-02-01T19:08:13Z

@singlis thank you for doing this! I left few comments regarding tests and small improvements. #Resolved

…notone

- Added Predictor tests that use the monotone constraints for LightGBM, LightGBMMC, and LightGBMR. - Added test to confirm that the monotonic trends work as expected - Added additional tests based upon review feedback.

justinormont · 2019-02-05T07:15:25Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+                // set the same constraint for all features.
+                if (subArguments.Length == 1)
+                {
+                    for(int i = 0; i < featureCount; i++)


Suggested change

for(int i = 0; i < featureCount; i++)

for (int i = 0; i < featureCount; i++)

``` #Resolved

thanks Justin - Ill format the files.
#Resolved

justinormont · 2019-02-05T07:15:34Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+
+                // Process each range
+                foreach(var indexRange in indexRanges)
+                   for(int i  = indexRange.min; i <= indexRange.max; ++i)


Suggested change

for(int i = indexRange.min; i <= indexRange.max; ++i)

for (int i = indexRange.min; i <= indexRange.max; ++i)

``` #Resolved

justinormont · 2019-02-05T07:16:17Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+                }
+
+                // Process each range
+                foreach(var indexRange in indexRanges)


Suggested change

foreach(var indexRange in indexRanges)

foreach (var indexRange in indexRanges)

``` #Resolved

justinormont · 2019-02-05T07:16:30Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

+            var negativeTestData = GenerateTestFeatures(1, numFeatures);
+
+            var lastValue = float.MinValue;
+            foreach(var x in positiveTestData)


Suggested change

foreach(var x in positiveTestData)

foreach (var x in positiveTestData)

``` #Resolved

justinormont · 2019-02-05T07:16:43Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

+            }
+
+            lastValue = float.MaxValue;
+            foreach(var x in negativeTestData)


Suggested change

foreach(var x in negativeTestData)

foreach (var x in negativeTestData)

``` #Resolved

justinormont · 2019-02-05T07:20:26Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+            // neg:6-7
+            // Since the argument is a string array, mulitple ranges can be specified.
+            // In the event of the same index being specified twice, the last one wins.
+            // The end result is a string array of 1s,0s, and -1s that should be equal


Suggested change

// The end result is a string array of 1s,0s, and -1s that should be equal

// The end result is a string array of 1s, 0s, and -1s that should be equal

``` #Resolved

justinormont · 2019-02-05T07:31:00Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+                var subArguments = argument.Split(':');
+                if (subArguments.Length > 2)
+                    // Invalid argument, skip to the next argument
+                    continue;


We're currently silently ignoring the user's (bad) input:

if (subArguments.Length > 2) // Invalid argument, skip to the next argument continue;

Do we generally throw for bad input? #Resolved

justinormont · 2019-02-05T07:33:53Z

src/Microsoft.ML.LightGBM/LightGbmTrainerBase.cs

+                else if (keyword.Equals(negativeKeyword, StringComparison.OrdinalIgnoreCase))
+                    constraint = -1;
+                else
+                    continue;


We're also silently ignore the user input here. #Resolved

Thanks Justin, I am now throwing. #Resolved

justinormont

LGTM. Thanks for adding!
Minor feedback above.

codecov · 2019-02-05T08:01:23Z

Codecov Report

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

@@            Coverage Diff            @@
##             master    #2330   +/-   ##
=========================================
  Coverage          ?    71.3%           
=========================================
  Files             ?      785           
  Lines             ?   141185           
  Branches          ?    16128           
=========================================
  Hits              ?   100679           
  Misses            ?    36036           
  Partials          ?     4470

Flag	Coverage Δ
#Debug	`71.3% <97.52%> (?)`
#production	`67.63% <91.04%> (?)`
#test	`85.4% <100%> (?)`

Ivanidzo4ka · 2019-02-05T17:00:58Z

test/BaselineOutput/SingleDebug/LightGBMBinary/LightGBM-CV-breast-cancer.Monotonic-out.txt

+Not training a calibrator because it is not needed.
+TEST POSITIVE RATIO:	0.3702 (134.0/(134.0+228.0))
+Confusion table
+          ||======================


||====================== [](start = 10, length = 24)

Are they really different between release and debug, or we can move them to Common? #Resolved

They are not different. I have another PR (not out yet) that moves all LightGBM tests to common, so I put these in release and debug for now and planned on moving on the next PR.
#Resolved

I will go ahead and move these to Common
#Resolved

If it's not hard for you. It just if you add them here, and then move them to Common directory, we just increase size of our git repo, without any reason whatsoever.

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

- Now throws when an invalid argument is parsed. - Updated tests

Ivanidzo4ka · 2019-02-06T00:06:49Z

src/Microsoft.ML.LightGBM/LightGbmArguments.cs

+            "first three features and a negative constraint to the 4th and 6th feature. If feature index is not specified, " +
+            "then no constraint will be applied. The keyword of 'pos' or 'neg' without a range will apply the constraint to all features.",
+            ShortName="mc")]
+        public string[] MonotoneConstraints;


MonotoneConstraints [](start = 24, length = 19)

Sorry what I bring it on 9th iteration, but just curious, wouldn't it be easier to have two options, one is positive monotone constraints, and other negative with types int[]?

or array of Range, or what class we use in textloader to specify range of columns.

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

Using the TextLoader's text-based ranges offers interesting abilities, like "~" and "*". For example setting "pos:3-*" so the user doesn't need to know the total number of slots.

The current method is interesting as it allows for setting everything positive, then some negative "pos neg:5-10". The gain here is the user doesn't need to know the total number of slots.

I don't see a shortcut to set positive on all except for a few: "pos neutral:5-10". If you know the total number of slots, you could do "pos:0-4,11-547542".

So initially I did not use TextLoader Range as it looked to contain properties specific to Text Loading -- however, your comments resonated with me so I have made some changes:

I created a new Range class in Microsoft.ML.Core - this is meant to be a more generic Range class that can be used by other Arguments if a range is needed. Tests were added as well.

I removed MonotonicConstraints as an argument and added MonotonicPositive and MonotonicNegative - these are now Range arrays.

I updated tests and baseline files to reflect these changes.

Overall I like this better as it follows the range format that is already used for text loader. If you want to select all features you can use the * and set the range to be 0-*. Short names for these arguments are mp and mn.

Take a look - Im curious to what you think. There maybe an opportunity to merge the Microsoft.ML.Core Range and the TextLoader Range, but I think that should be a different discussion.

In reply to: 254097097 [](ancestors = 254097097,254096866)

I didn't expect you to take me seriously on this proposal, I was just thinking out loud.
I'm fine with changes, but I would like @TomFinley to give his opinion regarding new Range class, and how it fit in our API story.

Thanks @Ivanidzo4ka, it looks this PR fails .netcore30 builds because there is now a System.Range and that is causing some compiler confusion as to which one is being referenced. So I am curious as to what @TomFinley has to say.

Ivanidzo4ka

singlis · 2019-02-06T02:49:31Z

No I think exposing Random Forest should be done in a separate PR.

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

MonotonicNegative (based upon feedback). - Added a Range class to support specifying ranges similar to TextLoader. - Updated unit tests, updated entry points.

…notone

singlis · 2019-02-06T22:14:46Z

test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs

@@ -249,7 +249,8 @@ private string GetBuildPrefix()
 #endif
        }

-        [Fact(Skip = "Execute this test if you want to regenerate the core_manifest and core_ep_list files")]
+//        [Fact(Skip = "Execute this test if you want to regenerate the core_manifest and core_ep_list files")]
+        [Fact]


Ill revert this.

singlis · 2019-02-06T22:16:46Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

+            var mlContext = new MLContext();
+            var dataView = GetRegressionPipeline();
+            var trainer = mlContext.Regression.Trainers.LightGbm(new Options() { 
+                    MonotonicPositive = new Range[] { new Range(0,null) }, 


null [](start = 66, length = 4)

Adding null here was a test, I will revert this.

Ivanidzo4ka · 2019-02-06T22:30:42Z

test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs

+        public void LightGBMMonotonicArgumentExtendToEnd()
+        {
+            Dictionary<string, object> options = new Dictionary<string, object>();
+            options["monotone_constraints"] = (positive: new Range[] { new Range(0, null) }, negative: new Range[] { });


negative: new Range[] { } [](start = 93, length = 25)

it looks weird to specify empty range. Is it necessary?

I am short circuiting things here for testing. This sets the monotone_constraints parameter in the options dictionary so that I can call ExpandMonotoneConstraints to test the output. But as for the user - they dont have to specify both, neither from command line usage or api usage.

Ivanidzo4ka · 2019-02-06T22:33:02Z

src/Microsoft.ML.Core/Data/Range.cs

+{
+    public class Range
+    {
+        public Range() { }


Range [](start = 15, length = 5)

How it's related to Range defined in TextLoader?

eerhardt · 2019-02-07T18:18:20Z

src/Microsoft.ML.Core/Data/Range.cs

+
+namespace Microsoft.ML.Data
+{
+    public class Range


We can't be adding a public type named Range. It conflicts with the new System.Range.

dotnet/designs#48

This would be equivalent to naming one of our new classes String or Object.

eerhardt

Please rename the Range type, or move it to be nested in some other class, like we do elsewhere.

singlis self-assigned this Jan 30, 2019

singlis requested review from codemzs, Ivanidzo4ka, wschin and sfilipi January 30, 2019 19:37

wschin reviewed Jan 30, 2019

View reviewed changes

justinormont reviewed Jan 30, 2019

View reviewed changes

singlis added 3 commits January 31, 2019 10:11

Fixing entry points, adding test to confirm constraints

3c451f4

Merge remote-tracking branch 'origin/master' into singlis/lightgbm-mo…

28ad5cb

…notone

...updating...

81d29a9

Ivanidzo4ka reviewed Feb 1, 2019

View reviewed changes

singlis added 3 commits February 1, 2019 11:27

Merge remote-tracking branch 'origin/master' into singlis/lightgbm-mo…

764ee16

…notone

- Updated the arguments to set all features as negative or positive

7e2df45

- Added Predictor tests that use the monotone constraints for LightGBM, LightGBMMC, and LightGBMR. - Added test to confirm that the monotonic trends work as expected - Added additional tests based upon review feedback.

..updating entry points and test files

6c819d4

justinormont reviewed Feb 5, 2019

View reviewed changes

- Formatting files

136bc38

justinormont reviewed Feb 5, 2019

View reviewed changes

justinormont approved these changes Feb 5, 2019

View reviewed changes

Ivanidzo4ka reviewed Feb 5, 2019

View reviewed changes

- Moved test files to Common

1e481e8

- Now throws when an invalid argument is parsed. - Updated tests

Ivanidzo4ka reviewed Feb 6, 2019

View reviewed changes

Ivanidzo4ka approved these changes Feb 6, 2019

View reviewed changes

singlis added 2 commits February 6, 2019 14:06

- Converted the MonotonicConstraints parameter to MonotonicPositive and

b22a303

MonotonicNegative (based upon feedback). - Added a Range class to support specifying ranges similar to TextLoader. - Updated unit tests, updated entry points.

Merge remote-tracking branch 'origin/master' into singlis/lightgbm-mo…

e391cbc

…notone

singlis commented Feb 6, 2019

View reviewed changes

Formatting changes

0231e6c

Ivanidzo4ka reviewed Feb 6, 2019

View reviewed changes

eerhardt reviewed Feb 7, 2019

View reviewed changes

eerhardt requested changes Feb 7, 2019

View reviewed changes

singlis mentioned this pull request Feb 7, 2019

Monotone_constraints in LightGbm #1651

Closed

singlis closed this Feb 7, 2019

ghost locked as resolved and limited conversation to collaborators Mar 25, 2022

	for(int i = 0; i < featureCount; i++)
	for (int i = 0; i < featureCount; i++)
	``` #Resolved

	for(int i = indexRange.min; i <= indexRange.max; ++i)
	for (int i = indexRange.min; i <= indexRange.max; ++i)
	``` #Resolved

	foreach(var indexRange in indexRanges)
	foreach (var indexRange in indexRanges)
	``` #Resolved

	foreach(var x in positiveTestData)
	foreach (var x in positiveTestData)
	``` #Resolved

	foreach(var x in negativeTestData)
	foreach (var x in negativeTestData)
	``` #Resolved

	// The end result is a string array of 1s,0s, and -1s that should be equal
	// The end result is a string array of 1s, 0s, and -1s that should be equal
	``` #Resolved

Monotone constraint support for LightGBM #2330

Monotone constraint support for LightGBM #2330

Conversation

singlis commented Jan 30, 2019

wschin Jan 30, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Jan 30, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinormont Feb 2, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont commented Jan 31, 2019 • edited by singlis Loading

Ivanidzo4ka Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ivanidzo4ka Feb 1, 2019 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka commented Feb 1, 2019 • edited by singlis Loading

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

singlis Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

justinormont Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

singlis Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

justinormont left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 5, 2019 • edited Loading

Codecov Report

Ivanidzo4ka Feb 5, 2019 • edited by singlis Loading

Choose a reason for hiding this comment

singlis Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

singlis Feb 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinormont Feb 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singlis Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Ivanidzo4ka left a comment

Choose a reason for hiding this comment

singlis commented Feb 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singlis Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eerhardt left a comment

Choose a reason for hiding this comment

wschin Jan 30, 2019 •

edited by singlis

Loading

justinormont Jan 30, 2019 •

edited by singlis

Loading

justinormont Feb 2, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont commented Jan 31, 2019 •

edited by singlis

Loading

Ivanidzo4ka Feb 1, 2019 •

edited

Loading

Ivanidzo4ka Feb 1, 2019 •

edited

Loading

Ivanidzo4ka Feb 1, 2019 •

edited

Loading

Ivanidzo4ka Feb 1, 2019 •

edited

Loading

Ivanidzo4ka commented Feb 1, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

singlis Feb 5, 2019 •

edited

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

justinormont Feb 5, 2019 •

edited by singlis

Loading

singlis Feb 5, 2019 •

edited

Loading

codecov bot commented Feb 5, 2019 •

edited

Loading

Ivanidzo4ka Feb 5, 2019 •

edited by singlis

Loading

singlis Feb 5, 2019 •

edited

Loading

singlis Feb 5, 2019 •

edited

Loading

justinormont Feb 6, 2019 •

edited

Loading

singlis Feb 7, 2019 •

edited

Loading

singlis Feb 7, 2019 •

edited

Loading