-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Support for custom metrics reported in the Benchmarks #735
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
Changes from all commits
f18f927
058ab89
c09024f
1ea36e2
a6ff27a
da080f1
4e90e06
fe38aeb
282b088
8c4e9b9
5bfa492
ec3df9f
f209d96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using BenchmarkDotNet.Attributes; | ||
using BenchmarkDotNet.Columns; | ||
using BenchmarkDotNet.Reports; | ||
using BenchmarkDotNet.Running; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Text; | ||
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
public abstract class WithExtraMetrics | ||
{ | ||
protected abstract IEnumerable<Metric> GetMetrics(); | ||
|
||
/// <summary> | ||
/// this method is executed after running the benchmrks | ||
/// we use it as hack to simply print to console so ExtraMetricColumn can parse the output | ||
/// </summary> | ||
[GlobalCleanup] | ||
public void ReportMetrics() | ||
{ | ||
foreach (var metric in GetMetrics()) | ||
{ | ||
Console.WriteLine(metric.ToParsableString()); | ||
} | ||
} | ||
} | ||
|
||
public class ExtraMetricColumn : IColumn | ||
{ | ||
public string ColumnName => "Extra Metric"; | ||
public string Id => nameof(ExtraMetricColumn); | ||
public string Legend => "Value of the provided extra metric"; | ||
public bool IsNumeric => true; | ||
public bool IsDefault(Summary summary, BenchmarkCase benchmark) => true; | ||
public bool IsAvailable(Summary summary) => true; | ||
public bool AlwaysShow => true; | ||
public ColumnCategory Category => ColumnCategory.Custom; | ||
public int PriorityInCategory => 1; | ||
public UnitType UnitType => UnitType.Dimensionless; | ||
public string GetValue(Summary summary, BenchmarkCase benchmark) => GetValue(summary, benchmark, null); | ||
public override string ToString() => ColumnName; | ||
|
||
public string GetValue(Summary summary, BenchmarkCase benchmark, ISummaryStyle style) | ||
{ | ||
if (!summary.HasReport(benchmark)) | ||
return "-"; | ||
|
||
var results = summary[benchmark].ExecuteResults; | ||
if (results.Count != 1) | ||
return "-"; | ||
|
||
var result = results.Single(); | ||
var buffer = new StringBuilder(); | ||
|
||
foreach (var line in result.ExtraOutput) | ||
{ | ||
if (Metric.TryParse(line, out Metric metric)) | ||
{ | ||
if (buffer.Length > 0) | ||
buffer.Append(", "); | ||
|
||
buffer.Append(metric.ToColumnValue()); | ||
} | ||
} | ||
|
||
return buffer.Length > 0 ? buffer.ToString() : "-"; | ||
} | ||
} | ||
|
||
public struct Metric | ||
{ | ||
private const string Prefix = "// Metric"; | ||
private const char Separator = '#'; | ||
|
||
public string Name { get; } | ||
public string Value { get; } | ||
|
||
public Metric(string name, string value) : this() | ||
{ | ||
Name = name; | ||
Value = value; | ||
} | ||
|
||
public string ToColumnValue() | ||
=> $"{Name}: {Value}"; | ||
|
||
public string ToParsableString() | ||
=> $"{Prefix} {Separator} {Name} {Separator} {Value}"; | ||
|
||
public static bool TryParse(string line, out Metric metric) | ||
{ | ||
metric = default; | ||
|
||
if (!line.StartsWith(Prefix)) | ||
return false; | ||
|
||
var splitted = line.Split(Separator); | ||
|
||
metric = new Metric(splitted[1].Trim(), splitted[2].Trim()); | ||
|
||
return true; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using BenchmarkDotNet.Extensions; | ||
using BenchmarkDotNet.Toolchains; | ||
using BenchmarkDotNet.Toolchains.CsProj; | ||
using System; | ||
using System.IO; | ||
using System.Linq; | ||
|
||
namespace Microsoft.ML.Benchmarks.Harness | ||
{ | ||
/// <summary> | ||
/// to avoid side effects of benchmarks affect each other BenchmarkDotNet runs every benchmark in a standalone, dedicated process | ||
/// however to do that it needs to be able to create, build and run new executable | ||
/// | ||
/// the problem with ML.NET is that it has native dependencies, which are NOT copied by MSBuild to the output folder | ||
/// in case where A has native dependency and B references A | ||
/// | ||
/// this is why this class exists: to copy the native dependencies to folder with .exe | ||
/// </summary> | ||
public class ProjectGenerator : CsProjGenerator | ||
{ | ||
public ProjectGenerator(string targetFrameworkMoniker) : base(targetFrameworkMoniker, platform => platform.ToConfig(), null) | ||
{ | ||
} | ||
|
||
protected override void CopyAllRequiredFiles(ArtifactsPaths artifactsPaths) | ||
{ | ||
base.CopyAllRequiredFiles(artifactsPaths); | ||
|
||
CopyMissingNativeDependencies(artifactsPaths); | ||
} | ||
|
||
private void CopyMissingNativeDependencies(ArtifactsPaths artifactsPaths) | ||
{ | ||
var foldeWithAutogeneratedExe = Path.GetDirectoryName(artifactsPaths.ExecutablePath); | ||
var folderWithNativeDependencies = Path.GetDirectoryName(typeof(ProjectGenerator).Assembly.Location); | ||
|
||
foreach (var nativeDependency in Directory | ||
.EnumerateFiles(folderWithNativeDependencies) | ||
.Where(fileName => ContainsWithIgnoreCase(fileName, "native"))) | ||
{ | ||
File.Copy( | ||
sourceFileName: nativeDependency, | ||
destFileName: Path.Combine(foldeWithAutogeneratedExe, Path.GetFileName(nativeDependency)), | ||
overwrite: true); | ||
} | ||
} | ||
|
||
bool ContainsWithIgnoreCase(string text, string word) => text != null && text.IndexOf(word, StringComparison.InvariantCultureIgnoreCase) >= 0; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,13 @@ | |
using BenchmarkDotNet.Diagnosers; | ||
using BenchmarkDotNet.Jobs; | ||
using BenchmarkDotNet.Running; | ||
using BenchmarkDotNet.Columns; | ||
using BenchmarkDotNet.Reports; | ||
using BenchmarkDotNet.Toolchains.InProcess; | ||
using BenchmarkDotNet.Toolchains; | ||
using BenchmarkDotNet.Toolchains.CsProj; | ||
using BenchmarkDotNet.Toolchains.DotNetCli; | ||
using Microsoft.ML.Benchmarks.Harness; | ||
using System.Globalization; | ||
using System.IO; | ||
using Microsoft.ML.Models; | ||
using System.Threading; | ||
|
||
namespace Microsoft.ML.Benchmarks | ||
{ | ||
|
@@ -28,52 +30,33 @@ static void Main(string[] args) | |
private static IConfig CreateCustomConfig() | ||
=> DefaultConfig.Instance | ||
.With(Job.Default | ||
.WithWarmupCount(1) // for our time consuming benchmarks 1 warmup iteration is enough | ||
.WithMaxIterationCount(20) | ||
.With(InProcessToolchain.Instance)) | ||
.With(new ClassificationMetricsColumn("AccuracyMacro", "Macro-average accuracy of the model")) | ||
.With(CreateToolchain())) | ||
.With(new ExtraMetricColumn()) | ||
.With(MemoryDiagnoser.Default); | ||
|
||
internal static string GetDataPath(string name) | ||
=> Path.GetFullPath(Path.Combine(_dataRoot, name)); | ||
|
||
static readonly string _dataRoot; | ||
static Program() | ||
/// <summary> | ||
/// we need our own toolchain because MSBuild by default does not copy recursive native dependencies to the output | ||
/// </summary> | ||
private static IToolchain CreateToolchain() | ||
{ | ||
var currentAssemblyLocation = new FileInfo(typeof(Program).Assembly.Location); | ||
var rootDir = currentAssemblyLocation.Directory.Parent.Parent.Parent.Parent.FullName; | ||
_dataRoot = Path.Combine(rootDir, "test", "data"); | ||
var csProj = CsProjCoreToolchain.Current.Value; | ||
var tfm = NetCoreAppSettings.Current.Value.TargetFrameworkMoniker; | ||
|
||
return new Toolchain( | ||
tfm, | ||
new ProjectGenerator(tfm), | ||
csProj.Builder, | ||
csProj.Executor); | ||
} | ||
} | ||
|
||
public class ClassificationMetricsColumn : IColumn | ||
{ | ||
private readonly string _metricName; | ||
private readonly string _legend; | ||
|
||
public ClassificationMetricsColumn(string metricName, string legend) | ||
internal static string GetInvariantCultureDataPath(string name) | ||
{ | ||
_metricName = metricName; | ||
_legend = legend; | ||
} | ||
|
||
public string ColumnName => _metricName; | ||
public string Id => _metricName; | ||
public string Legend => _legend; | ||
public bool IsNumeric => true; | ||
public bool IsDefault(Summary summary, BenchmarkCase benchmark) => true; | ||
public bool IsAvailable(Summary summary) => true; | ||
public bool AlwaysShow => true; | ||
public ColumnCategory Category => ColumnCategory.Custom; | ||
public int PriorityInCategory => 1; | ||
public UnitType UnitType => UnitType.Dimensionless; | ||
// enforce Neutral Language as "en-us" because the input data files use dot as decimal separator (and it fails for cultures with ",") | ||
Thread.CurrentThread.CurrentCulture = CultureInfo.InvariantCulture; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line of code is a bit surprising in a method that is supposed to return a data path. Maybe it would be better to do this in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eerhardt I agree that I am breaking CQRS here. My only excuse is that I have named the method I was thinking about moving it to a I also wonder how ML.NET samples deal with the culture info problem. Does anybody know? |
||
|
||
public string GetValue(Summary summary, BenchmarkCase benchmark, ISummaryStyle style) | ||
{ | ||
var property = typeof(ClassificationMetrics).GetProperty(_metricName); | ||
return property.GetValue(StochasticDualCoordinateAscentClassifierBench.s_metrics).ToString(); | ||
return Path.Combine(Path.GetDirectoryName(typeof(Program).Assembly.Location), "Input", name); | ||
} | ||
public string GetValue(Summary summary, BenchmarkCase benchmark) => GetValue(summary, benchmark, null); | ||
|
||
public override string ToString() => ColumnName; | ||
} | ||
} |
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.
nit: Would it improve perf to set
var metrics = GetMetrics();
right before the foreach loop and then write the condition asvar metric in metrics
? Not sure...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.
@briancylui no, it would not.
Whenever you are not sure about something you can benchmark it with BenchmarkDotNet ;)