Skip to content

Fix SupportedMetric.ByName() method #280

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
merged 6 commits into from
Jun 1, 2018
Merged

Fix SupportedMetric.ByName() method #280

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08e2c73
Fix for SupportedMetric.ByName() method. Include new unit test for fu…
george-microsoft May 31, 2018
ea8f149
Fix for SupportedMetric.ByName() method. Include new unit test for fu…
george-microsoft May 31, 2018
9522d1b
Merge branch 'FixSupportedMetricByNameMethod' of https://github.com/g…
george-microsoft May 31, 2018
94f29bd
Fix for SupportedMetric.ByName() method. Include new unit test for fu…
george-microsoft May 31, 2018
f729a71
Merge branch 'FixSupportedMetricByNameMethod' of https://github.com/g…
george-microsoft May 31, 2018
75f76f1
Removed unnecessary field filter, per review comment.
george-microsoft Jun 1, 2018
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
8 changes: 5 additions & 3 deletions src/Microsoft.ML.PipelineInference/AutoInference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,14 @@ private SupportedMetric(string name, bool isMaximizing)
public static SupportedMetric ByName(string name)
{
var fields =
typeof(SupportedMetric).GetMembers(BindingFlags.Static | BindingFlags.Public)
typeof(SupportedMetric).GetFields(BindingFlags.Static | BindingFlags.Public)
.Where(s => s.MemberType == MemberTypes.Field);
Copy link
Contributor

Choose a reason for hiding this comment

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

.Where(s => s.MemberType == MemberTypes.Field); [](start = 20, length = 47)

Given that you're now using GetFields is it necessary to have this filter on s.MemberType == MemberTypes.Field any longer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I can likely remove it now.


foreach (var field in fields)
{
if (name.Equals(field.Name, StringComparison.OrdinalIgnoreCase))
return (SupportedMetric)typeof(SupportedMetric).GetField(field.Name).GetValue(null);
var metric = (SupportedMetric)field.GetValue(Auc);
if (name.Equals(metric.Name, StringComparison.OrdinalIgnoreCase))
return metric;
}
throw new NotSupportedException($"Metric '{name}' not supported.");
}
Expand Down
22 changes: 22 additions & 0 deletions test/Microsoft.ML.Predictor.Tests/TestAutoInference.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Linq;
using System.Collections.Generic;
using Newtonsoft.Json.Linq;
using Microsoft.ML.Runtime.Data;
using Microsoft.ML.Runtime.EntryPoints;
Expand Down Expand Up @@ -411,6 +412,27 @@ public void TestPipelineNodeCloning()
}
}

[Fact]
public void TestSupportedMetricsByName()
{
var names = new List<string>()
{
AutoInference.SupportedMetric.AccuracyMacro.Name,
AutoInference.SupportedMetric.AccuracyMicro.Name,
AutoInference.SupportedMetric.Auc.Name,
AutoInference.SupportedMetric.AuPrc.Name,
AutoInference.SupportedMetric.Dbi.Name,
AutoInference.SupportedMetric.F1.Name,
AutoInference.SupportedMetric.LogLossReduction.Name
};

foreach (var name in names)
{
var metric = AutoInference.SupportedMetric.ByName(name);
Assert.Equal(metric.Name, name);
}
}

[Fact]
public void TestHyperparameterFreezing()
{
Expand Down