Skip to content

Commit 4a7e68d

Browse files
TomFinleyTomFinley
committed
Eric comments
1 parent 0a40d88 commit 4a7e68d

File tree

20 files changed

+46
-88
lines changed

20 files changed

+46
-88
lines changed

‎src/Microsoft.ML.Api/TypedCursor.cs

-39
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,6 @@ public ICursor GetRootCursor()
528528
/// </summary>
529529
public static class CursoringUtils
530530
{
531-
private const string NeedEnvObsoleteMessage = "This method is obsolete. Please use the overload that takes an additional 'env' argument. An environment can be created via new LocalEnvironment().";
532-
533531
/// <summary>
534532
/// Generate a strongly-typed cursorable wrapper of the <see cref="IDataView"/>.
535533
/// </summary>
@@ -550,24 +548,6 @@ public static ICursorable<TRow> AsCursorable<TRow>(this IDataView data, IHostEnv
550548
return TypedCursorable<TRow>.Create(env, data, ignoreMissingColumns, schemaDefinition);
551549
}
552550

553-
/// <summary>
554-
/// Generate a strongly-typed cursorable wrapper of the <see cref="IDataView"/>.
555-
/// </summary>
556-
/// <typeparam name="TRow">The user-defined row type.</typeparam>
557-
/// <param name="data">The underlying data view.</param>
558-
/// <param name="ignoreMissingColumns">Whether to ignore the case when a requested column is not present in the data view.</param>
559-
/// <param name="schemaDefinition">Optional user-provided schema definition. If it is not present, the schema is inferred from the definition of T.</param>
560-
/// <returns>The cursorable wrapper of <paramref name="data"/>.</returns>
561-
[Obsolete(NeedEnvObsoleteMessage)]
562-
public static ICursorable<TRow> AsCursorable<TRow>(this IDataView data, bool ignoreMissingColumns = false,
563-
SchemaDefinition schemaDefinition = null)
564-
where TRow : class, new()
565-
{
566-
// REVIEW: Take this as a parameter, or else make it a method on the context itself.
567-
var ml = new MLContext();
568-
return data.AsCursorable<TRow>(ml, ignoreMissingColumns, schemaDefinition);
569-
}
570-
571551
/// <summary>
572552
/// Convert an <see cref="IDataView"/> into a strongly-typed <see cref="IEnumerable{TRow}"/>.
573553
/// </summary>
@@ -589,24 +569,5 @@ public static IEnumerable<TRow> AsEnumerable<TRow>(this IDataView data, IHostEnv
589569
var engine = new PipeEngine<TRow>(env, data, ignoreMissingColumns, schemaDefinition);
590570
return engine.RunPipe(reuseRowObject);
591571
}
592-
593-
/// <summary>
594-
/// Convert an <see cref="IDataView"/> into a strongly-typed <see cref="IEnumerable{TRow}"/>.
595-
/// </summary>
596-
/// <typeparam name="TRow">The user-defined row type.</typeparam>
597-
/// <param name="data">The underlying data view.</param>
598-
/// <param name="reuseRowObject">Whether to return the same object on every row, or allocate a new one per row.</param>
599-
/// <param name="ignoreMissingColumns">Whether to ignore the case when a requested column is not present in the data view.</param>
600-
/// <param name="schemaDefinition">Optional user-provided schema definition. If it is not present, the schema is inferred from the definition of T.</param>
601-
/// <returns>The <see cref="IEnumerable{TRow}"/> that holds the data in <paramref name="data"/>. It can be enumerated multiple times.</returns>
602-
[Obsolete(NeedEnvObsoleteMessage)]
603-
public static IEnumerable<TRow> AsEnumerable<TRow>(this IDataView data, bool reuseRowObject,
604-
bool ignoreMissingColumns = false, SchemaDefinition schemaDefinition = null)
605-
where TRow : class, new()
606-
{
607-
// REVIEW: Take this as a parameter, or else make it a method on the context itself.
608-
var ml = new MLContext();
609-
return data.AsEnumerable<TRow>(ml, reuseRowObject, ignoreMissingColumns, schemaDefinition);
610-
}
611572
}
612573
}

‎src/Microsoft.ML.Core/ComponentModel/AssemblyLoadingUtils.cs

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
namespace Microsoft.ML.Runtime
1212
{
13+
[Obsolete("The usage for this is intended for the internal command line utilities and is not intended for anything related to the API. " +
14+
"Please consider another way of doing whatever it is you're attempting to accomplish.")]
1315
[BestFriend]
1416
internal static class AssemblyLoadingUtils
1517
{

‎src/Microsoft.ML.Core/Properties/AssemblyInfo.cs

-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.InferenceTesting" + PublicKey.TestValue)]
1313
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.StaticPipelineTesting" + PublicKey.TestValue)]
1414
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.OnnxTransformTest" + PublicKey.TestValue)]
15-
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.BenchMarks" + PublicKey.TestValue)]
1615

1716
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Legacy" + PublicKey.Value)]
1817
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Maml" + PublicKey.Value)]

‎src/Microsoft.ML.Data/Properties/AssemblyInfo.cs

-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Tests" + PublicKey.TestValue)]
1010
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.InferenceTesting" + PublicKey.TestValue)]
1111
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.OnnxTransformTest" + PublicKey.TestValue)]
12-
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.BenchMarks" + PublicKey.TestValue)]
1312

1413
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Legacy" + PublicKey.Value)]
1514
[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Maml" + PublicKey.Value)]

‎src/Microsoft.ML.Ensemble/OutputCombiners/BaseScalarStacking.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
namespace Microsoft.ML.Runtime.Ensemble.OutputCombiners
1111
{
12-
public abstract class BaseScalarStacking : BaseStacking<Single>
12+
internal abstract class BaseScalarStacking : BaseStacking<Single>
1313
{
1414
internal BaseScalarStacking(IHostEnvironment env, string name, ArgumentsBase args)
1515
: base(env, name, args)

‎src/Microsoft.ML.Ensemble/OutputCombiners/BaseStacking.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
namespace Microsoft.ML.Runtime.Ensemble.OutputCombiners
1717
{
1818
using ColumnRole = RoleMappedSchema.ColumnRole;
19-
public abstract class BaseStacking<TOutput> : IStackingTrainer<TOutput>
19+
internal abstract class BaseStacking<TOutput> : IStackingTrainer<TOutput>
2020
{
2121
public abstract class ArgumentsBase
2222
{

‎src/Microsoft.ML.Ensemble/OutputCombiners/MultiStacking.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ private static VersionInfo GetVersionInfo()
4141
[TlcModule.Component(Name = LoadName, FriendlyName = Stacking.UserName)]
4242
public sealed class Arguments : ArgumentsBase, ISupportMulticlassOutputCombinerFactory
4343
{
44-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
44+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
4545
[Argument(ArgumentType.Multiple, HelpText = "Base predictor for meta learning", ShortName = "bp", SortOrder = 50,
4646
Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureMultiClassClassifierTrainer))]
4747
[TGUI(Label = "Base predictor")]

‎src/Microsoft.ML.Ensemble/OutputCombiners/RegressionStacking.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ private static VersionInfo GetVersionInfo()
3939
[TlcModule.Component(Name = LoadName, FriendlyName = Stacking.UserName)]
4040
public sealed class Arguments : ArgumentsBase, ISupportRegressionOutputCombinerFactory
4141
{
42-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
42+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
4343
[Argument(ArgumentType.Multiple, HelpText = "Base predictor for meta learning", ShortName = "bp", SortOrder = 50,
4444
Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureRegressorTrainer))]
4545
[TGUI(Label = "Base predictor")]

‎src/Microsoft.ML.Ensemble/OutputCombiners/Stacking.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ private static VersionInfo GetVersionInfo()
3737
[TlcModule.Component(Name = LoadName, FriendlyName = UserName)]
3838
public sealed class Arguments : ArgumentsBase, ISupportBinaryOutputCombinerFactory
3939
{
40-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
40+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
4141
[Argument(ArgumentType.Multiple, HelpText = "Base predictor for meta learning", ShortName = "bp", SortOrder = 50,
4242
Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureBinaryClassifierTrainer))]
4343
[TGUI(Label = "Base predictor")]

‎src/Microsoft.ML.Ensemble/Trainer/Binary/EnsembleTrainer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public sealed class Arguments : ArgumentsBase
4848
[TGUI(Label = "Output combiner", Description = "Output combiner type")]
4949
public ISupportBinaryOutputCombinerFactory OutputCombiner = new MedianFactory();
5050

51-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
51+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
5252
[Argument(ArgumentType.Multiple, HelpText = "Base predictor type", ShortName = "bp,basePredictorTypes", SortOrder = 1, Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureBinaryClassifierTrainer))]
5353
public IComponentFactory<ITrainer<TScalarPredictor>>[] BasePredictors;
5454

‎src/Microsoft.ML.Ensemble/Trainer/Multiclass/MulticlassDataPartitionEnsembleTrainer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public sealed class Arguments : ArgumentsBase
4949
[TGUI(Label = "Output combiner", Description = "Output combiner type")]
5050
public ISupportMulticlassOutputCombinerFactory OutputCombiner = new MultiMedian.Arguments();
5151

52-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
52+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
5353
[Argument(ArgumentType.Multiple, HelpText = "Base predictor type", ShortName = "bp,basePredictorTypes", SortOrder = 1, Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureMultiClassClassifierTrainer))]
5454
public IComponentFactory<ITrainer<TVectorPredictor>>[] BasePredictors;
5555

‎src/Microsoft.ML.Ensemble/Trainer/Regression/RegressionEnsembleTrainer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public sealed class Arguments : ArgumentsBase
4343
[TGUI(Label = "Output combiner", Description = "Output combiner type")]
4444
public ISupportRegressionOutputCombinerFactory OutputCombiner = new MedianFactory();
4545

46-
// TODO: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
46+
// REVIEW: If we make this public again it should be an *estimator* of this type of predictor, rather than the (deprecated) ITrainer.
4747
[Argument(ArgumentType.Multiple, HelpText = "Base predictor type", ShortName = "bp,basePredictorTypes", SortOrder = 1, Visibility = ArgumentAttribute.VisibilityType.CmdLineOnly, SignatureType = typeof(SignatureRegressorTrainer))]
4848
public IComponentFactory<ITrainer<TScalarPredictor>>[] BasePredictors;
4949

‎src/Microsoft.ML.Legacy/AssemblyRegistration.cs

+2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ public static void RegisterAssemblies(IHostEnvironment environment)
2828
Contracts.Assert(_assemblyInitializer.Value);
2929
}
3030

31+
#pragma warning disable CS0618 // The legacy API that internally uses dependency injection for all calls will be deleted anyway.
3132
AssemblyLoadingUtils.RegisterCurrentLoadedAssemblies(environment);
33+
#pragma warning restore CS0618
3234
}
3335

3436
/// <summary>

‎src/Microsoft.ML.Maml/HelpCommand.cs

+2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ public void Run()
101101

102102
public void Run(int? columns)
103103
{
104+
#pragma warning disable CS0618 // The help command should be entirely within the command line anyway.
104105
AssemblyLoadingUtils.LoadAndRegister(_env, _extraAssemblies);
106+
#pragma warning restore CCS0618
105107

106108
using (var ch = _env.Start("Help"))
107109
using (var sw = new StringWriter(CultureInfo.InvariantCulture))

‎src/Microsoft.ML.Maml/MAML.cs

+2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ private static int MainWithProgress(string args)
5858
string currentDirectory = Path.GetDirectoryName(typeof(Maml).Module.FullyQualifiedName);
5959

6060
using (var env = CreateEnvironment())
61+
#pragma warning disable CS0618 // This is the command line project, so the usage here is OK.
6162
using (AssemblyLoadingUtils.CreateAssemblyRegistrar(env, currentDirectory))
63+
#pragma warning restore CS0618
6264
using (var progressCancel = new CancellationTokenSource())
6365
{
6466
var progressTrackerTask = Task.Run(() => TrackProgress(env, progressCancel.Token));

‎src/Microsoft.ML.ResultProcessor/ResultProcessor.cs

+6
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,9 @@ public void GetDefaultSettingValues(IHostEnvironment env, string predictorName,
151151
/// <param name="extraAssemblies"></param>
152152
private Dictionary<string, string> GetDefaultSettings(IHostEnvironment env, string predictorName, string[] extraAssemblies = null)
153153
{
154+
#pragma warning disable CS0618 // The result processor is an internal command line processing utility anyway, so this is, while not great, OK.
154155
AssemblyLoadingUtils.LoadAndRegister(env, extraAssemblies);
156+
#pragma warning restore CS0618
155157

156158
var cls = env.ComponentCatalog.GetLoadableClassInfo<SignatureTrainer>(predictorName);
157159
if (cls == null)
@@ -1154,7 +1156,9 @@ public static int Main(string[] args)
11541156
{
11551157
string currentDirectory = Path.GetDirectoryName(typeof(ResultProcessor).Module.FullyQualifiedName);
11561158
using (var env = new ConsoleEnvironment(42))
1159+
#pragma warning disable CS0618 // The result processor is an internal command line processing utility anyway, so this is, while not great, OK.
11571160
using (AssemblyLoadingUtils.CreateAssemblyRegistrar(env, currentDirectory))
1161+
#pragma warning restore CS0618
11581162
return Main(env, args);
11591163
}
11601164

@@ -1197,7 +1201,9 @@ protected static void Run(IHostEnvironment env, string[] args)
11971201
if (cmd.IncludePerFoldResults)
11981202
cmd.PerFoldResultSeparator = "" + PredictionUtil.SepCharFromString(cmd.PerFoldResultSeparator);
11991203

1204+
#pragma warning disable CS0618 // The result processor is an internal command line processing utility anyway, so this is, while not great, OK.
12001205
AssemblyLoadingUtils.LoadAndRegister(env, cmd.ExtraAssemblies);
1206+
#pragma warning restore CS0618
12011207

12021208
if (cmd.Metrics.Length == 0)
12031209
cmd.Metrics = null;

‎src/Microsoft.ML.Sweeper/ConfigRunner.cs

+2
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ public virtual void Finish()
110110
string currentDirectory = Path.GetDirectoryName(typeof(ExeConfigRunnerBase).Module.FullyQualifiedName);
111111

112112
using (var ch = Host.Start("Finish"))
113+
#pragma warning disable CS0618 // As this deals with invoking command lines, this may be OK, though this code has some other problems.
113114
using (AssemblyLoadingUtils.CreateAssemblyRegistrar(Host, currentDirectory))
115+
#pragma warning restore CS0618
114116
{
115117
var runs = RunNums.ToArray();
116118
var args = Utils.BuildArray(RunNums.Count + 2,

‎test/Microsoft.ML.Benchmarks/Helpers/EnvironmentFactory.cs

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.ML.Core.Data;
66
using Microsoft.ML.Runtime;
77
using Microsoft.ML.Runtime.Data;
8+
using Microsoft.ML.Runtime.Training;
89
using Microsoft.ML.Transforms;
910

1011
namespace Microsoft.ML.Benchmarks
@@ -14,7 +15,7 @@ internal static class EnvironmentFactory
1415
internal static MLContext CreateClassificationEnvironment<TLoader, TTransformer, TTrainer>()
1516
where TLoader : IDataReader<IMultiStreamSource>
1617
where TTransformer : ITransformer
17-
where TTrainer : ITrainer
18+
where TTrainer : ITrainerEstimator<ISingleFeaturePredictionTransformer<IPredictor>, IPredictor>
1819
{
1920
var ctx = new MLContext();
2021
IHostEnvironment environment = ctx;
@@ -30,7 +31,7 @@ internal static MLContext CreateRankingEnvironment<TEvaluator, TLoader, TTransfo
3031
where TEvaluator : IEvaluator
3132
where TLoader : IDataReader<IMultiStreamSource>
3233
where TTransformer : ITransformer
33-
where TTrainer : ITrainer
34+
where TTrainer : ITrainerEstimator<ISingleFeaturePredictionTransformer<IPredictor>, IPredictor>
3435
{
3536
var ctx = new MLContext();
3637
IHostEnvironment environment = ctx;

‎test/Microsoft.ML.Benchmarks/KMeansAndLogisticRegressionBench.cs

+18-34
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,8 @@
33
// See the LICENSE file in the project root for more information.
44

55
using BenchmarkDotNet.Attributes;
6-
using Microsoft.ML.Runtime;
76
using Microsoft.ML.Runtime.Data;
87
using Microsoft.ML.Runtime.Internal.Calibration;
9-
using Microsoft.ML.Runtime.Learners;
10-
using Microsoft.ML.Trainers.KMeans;
11-
using Microsoft.ML.Transforms;
12-
using Microsoft.ML.Transforms.Categorical;
13-
using Microsoft.ML.Transforms.Normalizers;
148

159
namespace Microsoft.ML.Benchmarks
1610
{
@@ -21,14 +15,10 @@ public class KMeansAndLogisticRegressionBench
2115
[Benchmark]
2216
public ParameterMixingCalibratedPredictor TrainKMeansAndLR()
2317
{
24-
var env = new MLContext(seed: 1);
18+
var ml = new MLContext(seed: 1);
2519
// Pipeline
26-
var loader = TextLoader.ReadFile(env,
27-
new TextLoader.Arguments()
28-
{
29-
HasHeader = true,
30-
Separator = ",",
31-
Column = new[] {
20+
21+
var input = ml.Data.ReadFromTextFile(new[] {
3222
new TextLoader.Column("Label", DataKind.R4, 14),
3323
new TextLoader.Column("CatFeatures", DataKind.TX,
3424
new [] {
@@ -43,29 +33,23 @@ public ParameterMixingCalibratedPredictor TrainKMeansAndLR()
4333
new TextLoader.Range() { Min = 2, Max = 2 },
4434
new TextLoader.Range() { Min = 4, Max = 4 },
4535
new TextLoader.Range() { Min = 10, Max = 12 }
46-
})
47-
}
48-
}, new MultiFileSource(_dataPath));
49-
50-
IDataView trans = new OneHotEncodingEstimator(env, "CatFeatures").Fit(loader).Transform(loader);
51-
52-
trans = NormalizeTransform.CreateMinMaxNormalizer(env, trans, "NumFeatures");
53-
trans = new ConcatTransform(env, "Features", "NumFeatures", "CatFeatures").Transform(trans);
54-
trans = TrainAndScoreTransform.Create(env, new TrainAndScoreTransform.Arguments
36+
}),
37+
}, _dataPath, s =>
5538
{
56-
Trainer = ComponentFactoryUtils.CreateFromFunction(host =>
57-
new KMeansPlusPlusTrainer(host, "Features", advancedSettings: s =>
58-
{
59-
s.K = 100;
60-
})),
61-
FeatureColumn = "Features"
62-
}, trans);
63-
trans = new ConcatTransform(env, "Features", "Features", "Score").Transform(trans);
39+
s.HasHeader = true;
40+
s.Separator = ",";
41+
});
42+
43+
var estimatorPipeline = ml.Transforms.Categorical.OneHotEncoding("CatFeatures")
44+
.Append(ml.Transforms.Normalize("NumFeatures"))
45+
.Append(ml.Transforms.Concatenate("Features", "NumFeatures", "CatFeatures"))
46+
.Append(ml.Clustering.Trainers.KMeans("Features"))
47+
.Append(ml.Transforms.Concatenate("Features", "Features", "Score"))
48+
.Append(ml.BinaryClassification.Trainers.LogisticRegression(advancedSettings: args => { args.EnforceNonNegativity = true; args.OptTol = 1e-3f; }));
6449

65-
// Train
66-
var trainer = new LogisticRegression(env, "Label", "Features", advancedSettings: args => { args.EnforceNonNegativity = true; args.OptTol = 1e-3f; });
67-
var trainRoles = new RoleMappedData(trans, label: "Label", feature: "Features");
68-
return trainer.Train(trainRoles);
50+
var model = estimatorPipeline.Fit(input);
51+
// Return the last model in the chain.
52+
return model.LastTransformer.Model;
6953
}
7054
}
7155
}

0 commit comments

Comments
 (0)