-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Updates ml.net reference of LightGBM to version 2.2 #2448
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
Conversation
…#2446) - Updated the lightgbm parsing code to handle inf, -inf (now checks for contains rather than equals). - Additional updates for handling NaN - Moved all LightGBM baseline tests from SingleDebug/SingleRelease to Common. - Added Seed parameter to LightGBM arguments to support setting LightGBM's random seed.
FeatureColumn="Features", | ||
Seed=1, | ||
NThread=1, | ||
NumBoostRound=1})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctrl + K +D aka formatting #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report
@@ Coverage Diff @@
## master #2448 +/- ##
==========================================
+ Coverage 71.25% 71.26% +<.01%
==========================================
Files 797 797
Lines 141280 141293 +13
Branches 16115 16118 +3
==========================================
+ Hits 100675 100688 +13
+ Misses 36147 36144 -3
- Partials 4458 4461 +3
|
|
||
var trainedModel = pipe.Fit(preprocessedTrainData); | ||
var predicted = trainedModel.Transform(preprocessedTestData); | ||
var metrics = mlContext.MulticlassClassification.Evaluate(predicted); | ||
|
||
// First group of checks. They check if the overall prediction quality is ok using a test set. | ||
Assert.InRange(metrics.AccuracyMicro, expectedMicroAccuracy - .01, expectedMicroAccuracy + .01); | ||
Assert.InRange(metrics.AccuracyMacro, expectedMacroAccruacy - .01, expectedMicroAccuracy + .01); | ||
bool inRange = expectedValues.Any(exp => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use Assert.Equal
with a proper tolerance (e.g., 2) or Assert.InRange
? They automatically print out error information. #Pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to use Assert.InRange, however this is given a list of expected ranges that should match (i.e. micro and macro should match one of those ranges). Because of this, Assert.InRange will not work because it will fail on the first range that doesnt match.
In reply to: 254510687 [](ancestors = 254510687)
var expectedValues = new List<(double micro, double macro)>() | ||
{ | ||
(0.71304347826086956, 0.53197278911564627), | ||
(0.73304347826086956, 0.677551020408163) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so debug and release versions still differ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the benchmark differences that I am seeing between master and this change:
LightGBM Update
|
Here are the differences between master and this PR: Flight Data
|
Further benchmarks:
|
contains rather than equals).
Common.
LightGBM's random seed.