Skip to content
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

Jump to bottom

Added Done() call in BaseTestBaseline.Cleanup and added related fixes #4823

Merged
merged 5 commits into from
Mar 3, 2020
Merged

Added Done() call in BaseTestBaseline.Cleanup and added related fixes #4823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a37863
Added Done() call in BaseTestBaseline.Cleanup and added related fixes
harishsk Feb 10, 2020
bc5255a
Updated call stack collection mechanism to collect complete call stacks
harishsk Feb 11, 2020
3d9a630
Fixed defaults, documentation and added LdaWorkoutEstimatorCore to li…
harishsk Feb 13, 2020
073f247
Updated baseline and reduced precision for OneClassMatrixFactorizatio…
harishsk Feb 13, 2020
20aef91
Changed REVIEW tag to TODO TEST_STABILITY
harishsk Mar 3, 2020
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
14 changes: 7 additions & 7 deletions src/Microsoft.ML.Data/Prediction/Calibrator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1581,10 +1581,10 @@ internal sealed class FixedPlattCalibratorTrainer : ICalibratorTrainer
[TlcModule.Component(Name = "FixedPlattCalibrator", FriendlyName = "Fixed Platt Calibrator", Aliases = new[] { "FixedPlatt", "FixedSigmoid" })]
public sealed class Arguments : ICalibratorTrainerFactory
{
[Argument(ArgumentType.LastOccurrenceWins, HelpText = "The slope parameter of f(x) = 1 / (1 + exp(-slope * x + offset)", ShortName = "a")]
public Double Slope = 1;
[Argument(ArgumentType.LastOccurrenceWins, HelpText = "The slope parameter of f(x) = 1 / (1 + exp(slope * x + offset)", ShortName = "a")]
public Double Slope = -1;

[Argument(ArgumentType.LastOccurrenceWins, HelpText = "The offset parameter of f(x) = 1 / (1 + exp(-slope * x + offset)", ShortName = "b")]
[Argument(ArgumentType.LastOccurrenceWins, HelpText = "The offset parameter of f(x) = 1 / (1 + exp(slope * x + offset)", ShortName = "b")]
public Double Offset = 0;

public ICalibratorTrainer CreateComponent(IHostEnvironment env)
Expand Down Expand Up @@ -1618,7 +1618,7 @@ internal FixedPlattCalibratorTrainer(IHostEnvironment env, Arguments args)

///<summary>
/// The Platt calibrator calculates the probability following:
/// P(x) = 1 / (1 + exp(-<see cref="PlattCalibrator.Slope"/> * x + <see cref="PlattCalibrator.Offset"/>)
/// P(x) = 1 / (1 + exp(<see cref="PlattCalibrator.Slope"/> * x + <see cref="PlattCalibrator.Offset"/>)
/// </summary>.
public sealed class PlattCalibrator : ICalibrator, IParameterMixer, ICanSaveModel, ISingleCanSavePfa, ISingleCanSaveOnnx
{
Expand Down Expand Up @@ -2085,10 +2085,10 @@ public sealed class NoArgumentsInput : CalibrateInputBase

public sealed class FixedPlattInput : CalibrateInputBase
{
[Argument(ArgumentType.AtMostOnce, ShortName = "slope", HelpText = "The slope parameter of the calibration function 1 / (1 + exp(-slope * x + offset)", SortOrder = 1)]
public Double Slope = 1;
[Argument(ArgumentType.AtMostOnce, ShortName = "slope", HelpText = "The slope parameter of the calibration function 1 / (1 + exp(slope * x + offset)", SortOrder = 1)]
public Double Slope = -1;
Copy link

@yaeldekel yaeldekel Feb 11, 2020 •
edited by harishsk
Loading

Choose a reason for hiding this comment

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

Slope [](start = 26, length = 5)

FixedPlattCalibratorTrainer.Arguments also has a Slope field that defaults to 1. #Resolved


[Argument(ArgumentType.AtMostOnce, ShortName = "offset", HelpText = "The offset parameter of the calibration function 1 / (1 + exp(-slope * x + offset)", SortOrder = 3)]
[Argument(ArgumentType.AtMostOnce, ShortName = "offset", HelpText = "The offset parameter of the calibration function 1 / (1 + exp(slope * x + offset)", SortOrder = 3)]
public Double Offset = 0;
}

Expand Down
12 changes: 6 additions & 6 deletions test/BaselineOutput/Common/EntryPoints/core_manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1731,14 +1731,14 @@
{
"Name": "Slope",
"Type": "Float",
"Desc": "The slope parameter of the calibration function 1 / (1 + exp(-slope * x + offset)",
"Desc": "The slope parameter of the calibration function 1 / (1 + exp(slope * x + offset)",
"Aliases": [
"slope"
],
"Required": false,
"SortOrder": 1.0,
"IsNullable": false,
"Default": 1.0
"Default": -1.0
},
{
"Name": "Data",
Expand All @@ -1762,7 +1762,7 @@
{
"Name": "Offset",
"Type": "Float",
"Desc": "The offset parameter of the calibration function 1 / (1 + exp(-slope * x + offset)",
"Desc": "The offset parameter of the calibration function 1 / (1 + exp(slope * x + offset)",
"Aliases": [
"offset"
],
Expand Down Expand Up @@ -25072,19 +25072,19 @@
{
"Name": "Slope",
"Type": "Float",
"Desc": "The slope parameter of f(x) = 1 / (1 + exp(-slope * x + offset)",
"Desc": "The slope parameter of f(x) = 1 / (1 + exp(slope * x + offset)",
"Aliases": [
"a"
],
"Required": false,
"SortOrder": 150.0,
"IsNullable": false,
"Default": 1.0
"Default": -1.0
},
{
"Name": "Offset",
"Type": "Float",
"Desc": "The offset parameter of f(x) = 1 / (1 + exp(-slope * x + offset)",
"Desc": "The offset parameter of f(x) = 1 / (1 + exp(slope * x + offset)",
"Aliases": [
"b"
],
Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.ML.Core.Tests/UnitTests/TestEntryPoints.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,7 @@ public void EntryPointCalibrate()
var twiceCalibratedFfModel = Calibrate.Platt(Env,
new Calibrate.NoArgumentsInput() { Data = splitOutput.TestData[0], UncalibratedPredictorModel = calibratedFfModel }).PredictorModel;
var scoredFf = ScoreModel.Score(Env, new ScoreModel.Input() { Data = splitOutput.TestData[2], PredictorModel = twiceCalibratedFfModel }).ScoredData;
Done();
}

[Fact]
Expand Down
3 changes: 2 additions & 1 deletion test/Microsoft.ML.Predictor.Tests/TestPredictors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,7 @@ public void TestEnsembleCombiner()
};

CombineAndTestEnsembles(dataView, "pe", "oc=average", PredictionKind.BinaryClassification, predictors);
Done();
}

[X64Fact("x86 fails. Associated GitHubIssue: https://github.com/dotnet/machinelearning/issues/1216")]
Expand Down Expand Up @@ -941,7 +942,7 @@ private void CombineAndTestEnsembles(IDataView idv, string name, string options,
predGetters[i](ref preds[i]);
}
if (scores.All(s => !float.IsNaN(s)))
CompareNumbersWithTolerance(score, scores.Sum() / predCount);
CompareNumbersWithTolerance(score, scores.Sum() / predCount, digitsOfPrecision: 5);
Copy link
Contributor

@frank-dong-ms-zz frank-dong-ms-zz Feb 11, 2020 •
edited by harishsk
Loading

Choose a reason for hiding this comment

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

digitsOfPrecision [](start = 89, length = 17)

why we use less precise here? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default is 7 digits. All tests are not equally precise.

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

for (int i = 0; i < predCount; i++)
Assert.Equal(vectorScore.Length, vectorScores[i].Length);
for (int i = 0; i < vectorScore.Length; i++)
Expand Down
36 changes: 14 additions & 22 deletions test/Microsoft.ML.TestFramework/BaseTestBaseline.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.ExceptionServices;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
Expand Down Expand Up @@ -81,7 +81,7 @@ protected BaseTestBaseline(ITestOutputHelper output) : base(output)
protected IHostEnvironment Env => _env;
protected MLContext ML;
private bool _normal;
private readonly List<Exception> _failures = new List<Exception>();
private int _failures = 0;

protected override void Initialize()
{
Expand Down Expand Up @@ -126,6 +126,9 @@ protected override void Cleanup()
_normal ? "completed normally" : "aborted",
IsPassing ? "passed" : "failed");

if (!_normal)
Assert.Equal(0, _failures);

Contracts.AssertValue(LogWriter);
LogWriter.Dispose();
LogWriter = null;
Expand All @@ -135,7 +138,7 @@ protected override void Cleanup()

protected bool IsActive { get { return LogWriter != null; } }

protected bool IsPassing { get { return _failures.Count == 0; } }
protected bool IsPassing { get { return _failures == 0; } }

// Called by a test to signal normal completion. If this is not called before the
// TestScope is disposed, we assume the test was aborted.
Expand All @@ -145,18 +148,7 @@ protected void Done()
Contracts.Assert(!_normal, "Done() should only be called once!");
_normal = true;

switch (_failures.Count)
{
case 0:
break;

case 1:
ExceptionDispatchInfo.Capture(_failures[0]).Throw();
break;

default:
throw new AggregateException(_failures.ToArray());
}
Assert.Equal(0, _failures);
}

protected bool Check(bool f, string msg)
Expand All @@ -176,16 +168,16 @@ protected bool Check(bool f, string msg, params object[] args)
protected void Fail(string fmt, params object[] args)
{
Contracts.Assert(IsActive);
try
{
throw new InvalidOperationException(string.Format(fmt, args));
}
catch (Exception ex)
_failures++;
Log($"*** Failure #{_failures}: " + fmt, args);

var stackTrace = new StackTrace(true);
for (int i = 0; i < stackTrace.FrameCount; i++)
{
_failures.Add(ex);
var frame = stackTrace.GetFrame(i);
Log($"\t\t{frame.GetMethod()} {frame.GetFileName()} {frame.GetFileLineNumber()}");
}

Log("*** Failure: " + fmt, args);
}

protected void Log(string msg)
Expand Down
8 changes: 6 additions & 2 deletions test/Microsoft.ML.Tests/TrainerEstimators/MatrixFactorizationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,14 @@ public void OneClassMatrixFactorizationInMemoryDataZeroBaseIndex()
var testPrediction = model.Transform(testDataView);

var testResults = mlContext.Data.CreateEnumerable<OneClassMatrixElementZeroBasedForScore>(testPrediction, false).ToList();

// TODO TEST_STABILITY: We are seeing lower precision on non-Windows platforms
int precision = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? 5 : 3;

// Positive example (i.e., examples can be found in dataMatrix) is close to 1.
CompareNumbersWithTolerance(0.982391, testResults[0].Score, digitsOfPrecision: 5);
CompareNumbersWithTolerance(0.982391, testResults[0].Score, digitsOfPrecision: precision);
// Negative example (i.e., examples can not be found in dataMatrix) is close to 0.15 (specified by s.C = 0.15 in the trainer).
CompareNumbersWithTolerance(0.141411, testResults[1].Score, digitsOfPrecision: 5);
CompareNumbersWithTolerance(0.141411, testResults[1].Score, digitsOfPrecision: precision);
}

[MatrixFactorizationFact]
Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.ML.Tests/Transformers/TextFeaturizerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ public void LdaWorkout()
}

[Fact]
[Trait("Category", "SkipInCI")]
public void LdaWorkoutEstimatorCore()
Copy link
Contributor

Choose a reason for hiding this comment

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

LdaWorkoutEstimatorCore [](start = 20, length = 23)

I haven't see this fail in full build and CI build in last 14 days, this should be low fail rate tests and I think it is okay to left this one enabled, if we see this fails often later we can go back to this one

{
var ml = new MLContext(1);
Expand Down