-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Adding support for training metrics in PipelineSweeperMacro + new graph variable outputs #152
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 14 commits
05482eb
b789159
76dd270
483a50b
d42a522
fbee51c
0285c87
6f92693
e006ee1
90d96a3
1d1c303
ce9792f
a828266
c642f68
e066db0
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 |
---|---|---|
|
@@ -15,21 +15,34 @@ namespace Microsoft.ML.Runtime.PipelineInference | |
{ | ||
public static class AutoMlUtils | ||
{ | ||
public static AutoInference.RunSummary ExtractRunSummary(IHostEnvironment env, IDataView data, string metricColumnName) | ||
public static double ExtractValueFromIDV(IHostEnvironment env, IDataView result, string columnName) | ||
{ | ||
double metricValue = 0; | ||
int numRows = 0; | ||
var schema = data.Schema; | ||
schema.TryGetColumnIndex(metricColumnName, out var metricCol); | ||
Contracts.CheckValue(env, nameof(env)); | ||
env.CheckValue(result, nameof(result)); | ||
env.CheckNonEmpty(columnName, nameof(columnName)); | ||
|
||
using (var cursor = data.GetRowCursor(col => col == metricCol)) | ||
double outputValue = 0; | ||
var schema = result.Schema; | ||
if (!schema.TryGetColumnIndex(columnName, out var metricCol)) | ||
throw env.ExceptParam($"Schema does not contain column: {columnName}"); | ||
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.
Similar here, misuse of 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. Good catch. In switching over to ExceptParam I didn't change to reflect the new signature. Error on my part. |
||
|
||
using (var cursor = result.GetRowCursor(col => col == metricCol)) | ||
{ | ||
var getter = cursor.GetGetter<double>(metricCol); | ||
cursor.MoveNext(); | ||
getter(ref metricValue); | ||
bool moved = cursor.MoveNext(); | ||
env.Check(moved, "Expected an IDataView with a single row. Results dataset has no rows to extract."); | ||
getter(ref outputValue); | ||
env.Check(!cursor.MoveNext(), "Expected an IDataView with a single row. Results dataset has too many rows."); | ||
} | ||
|
||
return new AutoInference.RunSummary(metricValue, numRows, 0); | ||
return outputValue; | ||
} | ||
|
||
public static AutoInference.RunSummary ExtractRunSummary(IHostEnvironment env, IDataView result, string metricColumnName, IDataView trainResult = null) | ||
{ | ||
double testingMetricValue = ExtractValueFromIDV(env, result, metricColumnName); | ||
double trainingMetricValue = trainResult != null ? ExtractValueFromIDV(env, trainResult, metricColumnName) : double.MinValue; | ||
return new AutoInference.RunSummary(testingMetricValue, 0, 0, trainingMetricValue); | ||
} | ||
|
||
public static CommonInputs.IEvaluatorInput CloneEvaluatorInstance(CommonInputs.IEvaluatorInput evalInput) => | ||
|
@@ -618,5 +631,7 @@ public static Tuple<string, string[]>[] ConvertToSweepArgumentStrings(TlcModule. | |
} | ||
return results; | ||
} | ||
|
||
public static string GenerateOverallTrainingMetricVarName(Guid id) => $"Var_Training_OM_{id:N}"; | ||
} | ||
} |
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.
Note that this method takes two arguments in most usages. The first is the name of the variable, the second is the actual error message; we are putting the exception message into the variable name, which is surely a mistake.
I suppose the error is on the parameter
firstNodeInputs
, so maybenameof(firstNodeInputs)
.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.
Thanks, fixing.