From fc233aabca1b76097ce5bc0db9076e5440434069 Mon Sep 17 00:00:00 2001 From: Michael Sharp Date: Wed, 6 Oct 2021 19:33:03 -0700 Subject: [PATCH 1/6] Prediction engine no longer disposes model. --- src/Microsoft.ML.Data/Prediction/PredictionEngine.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs index b932369570..d053983def 100644 --- a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs +++ b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs @@ -139,7 +139,6 @@ public void Dispose() return; _disposer?.Invoke(); - (Transformer as IDisposable)?.Dispose(); _disposed = true; } From e09622f07350b34d485099b0fa5f7d756ee1443e Mon Sep 17 00:00:00 2001 From: Michael Sharp Date: Fri, 8 Oct 2021 19:37:15 -0700 Subject: [PATCH 2/6] Advanced options for the prediction engine. --- .../Model/ModelOperationsCatalog.cs | 17 ++++++++ .../Model/PredictionEngineExtensions.cs | 5 ++- .../Prediction/PredictionEngine.cs | 42 +++++++++++++++++-- .../PredictionEngine.cs | 37 ++++++++++++++++ .../Prediction.cs | 33 +++++++++++++++ 5 files changed, 129 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs index b449bdfa8e..dd56b21fec 100644 --- a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs +++ b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs @@ -333,5 +333,22 @@ public PredictionEngine CreatePredictionEngine(ITransfor return transformer.CreatePredictionEngine(_env, false, DataViewConstructionUtils.GetSchemaDefinition(_env, inputSchema)); } + + /// + /// Create a prediction engine for one-time prediction. + /// It's mainly used in conjunction with , + /// where input schema is extracted during loading the model. + /// + /// The class that defines the input data. + /// The class that defines the output data. + /// The transformer to use for prediction. + /// Advaned configuration options. + public PredictionEngine CreatePredictionEngine(ITransformer transformer, PredictionEngine.Options options) + where TSrc : class + where TDst : class, new() + { + return transformer.CreatePredictionEngine(_env, options.IgnoreMissingColumns, + options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnModelFile); + } } } diff --git a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs index 35d7fbf148..1e6e67c1bb 100644 --- a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs +++ b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs @@ -24,10 +24,11 @@ internal static class PredictionEngineExtensions /// . /// Additional settings of the input schema. /// Additional settings of the output schema. + /// Whether the prediction engine owns the model file and should dispose of it. public static PredictionEngine CreatePredictionEngine(this ITransformer transformer, - IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null) + IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownModelFile = true) where TSrc : class where TDst : class, new() - => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition); + => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition, ownModelFile); } } diff --git a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs index d053983def..79b5dfdc4d 100644 --- a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs +++ b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using Microsoft.ML.CommandLine; using Microsoft.ML.Data; using Microsoft.ML.Runtime; @@ -58,8 +59,8 @@ public sealed class PredictionEngine : PredictionEngineBase : IDisposable private readonly DataViewConstructionUtils.InputRow _inputRow; private readonly IRowReadableAs _outputRow; private readonly Action _disposer; + private readonly bool _ownModelFile; private bool _disposed; /// @@ -104,7 +106,7 @@ public abstract class PredictionEngineBase : IDisposable [BestFriend] private protected PredictionEngineBase(IHostEnvironment env, ITransformer transformer, bool ignoreMissingColumns, - SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null) + SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownModelFile = true) { Contracts.CheckValue(env, nameof(env)); env.AssertValue(transformer); @@ -112,6 +114,7 @@ private protected PredictionEngineBase(IHostEnvironment env, ITransformer transf var makeMapper = TransformerChecker(env, transformer); env.AssertValue(makeMapper); _inputRow = DataViewConstructionUtils.CreateInputRow(env, inputSchemaDefinition); + _ownModelFile = ownModelFile; PredictionEngineCore(env, _inputRow, makeMapper(_inputRow.Schema), ignoreMissingColumns, outputSchemaDefinition, out _disposer, out _outputRow); OutputSchema = Transformer.GetOutputSchema(_inputRow.Schema); } @@ -140,6 +143,9 @@ public void Dispose() _disposer?.Invoke(); + if (_ownModelFile) + (Transformer as IDisposable)?.Dispose(); + _disposed = true; } @@ -169,4 +175,34 @@ public TDst Predict(TSrc example) /// is reused. public abstract void Predict(TSrc example, ref TDst prediction); } + + public sealed class PredictionEngine + { + /// + /// Options for the as used in + /// [RandomizedPca(Options)](xref:Microsoft.ML.PcaCatalog.RandomizedPca(Microsoft.ML.AnomalyDetectionCatalog.AnomalyDetectionTrainers,Microsoft.ML.Trainers.RandomizedPcaTrainer.Options)). + /// + public sealed class Options + { + [Argument(ArgumentType.AtMostOnce, HelpText = "Whether to throw an error if a column exists in the output schema but not the output object.", ShortName = "ignore", SortOrder = 50)] + public bool IgnoreMissingColumns = Defaults.IgnoreMissingColumns; + + [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the input schema.", ShortName = "input", SortOrder = 50)] + public SchemaDefinition InputSchemaDefinition = Defaults.InputSchemaDefinition; + + [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the output schema.", ShortName = "output")] + public SchemaDefinition OutputSchemaDefinition = Defaults.OutputSchemaDefinition; + + [Argument(ArgumentType.AtMostOnce, HelpText = "Whether the prediction engine owns the model file and should dispose of it.", ShortName = "own")] + public bool OwnModelFile = Defaults.OwnModelFile; + + internal static class Defaults + { + public const bool IgnoreMissingColumns = true; + public const SchemaDefinition InputSchemaDefinition = null; + public const SchemaDefinition OutputSchemaDefinition = null; + public const bool OwnModelFile = true; + } + } + } } diff --git a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs index 7f75ad1d63..19d0cbb2cc 100644 --- a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs +++ b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs @@ -148,6 +148,15 @@ public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer { } + /// + /// Contructor for creating time series specific prediction engine. It allows the time series model to be updated with the observations + /// seen at prediction time via + /// + public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer, PredictionEngine.Options options) : + base(env, CloneTransformers(transformer), options.IgnoreMissingColumns, options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnModelFile) + { + } + internal DataViewRow GetStatefulRows(DataViewRow input, IRowToRowMapper mapper, IEnumerable activeColumns, List rows) { Contracts.CheckValue(input, nameof(input)); @@ -398,5 +407,33 @@ public static TimeSeriesPredictionEngine CreateTimeSeriesEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition); } + + /// + /// creates a prediction engine for a time series pipeline. + /// It updates the state of time series model with observations seen at prediction phase and allows checkpointing the model. + /// + /// Class describing input schema to the model. + /// Class describing the output schema of the prediction. + /// The time series pipeline in the form of a . + /// Usually + /// Advaned configuration options. + ///

Example code can be found by searching for TimeSeriesPredictionEngine in ML.NET.

+ /// + /// + /// + /// + /// + public static TimeSeriesPredictionEngine CreateTimeSeriesEngine(this ITransformer transformer, IHostEnvironment env, + PredictionEngine.Options options) + where TSrc : class + where TDst : class, new() + { + Contracts.CheckValue(env, nameof(env)); + env.CheckValue(options, nameof(options)); + return new TimeSeriesPredictionEngine(env, transformer, options); + } } } diff --git a/test/Microsoft.ML.IntegrationTests/Prediction.cs b/test/Microsoft.ML.IntegrationTests/Prediction.cs index 40df4c104b..d59fb11e81 100644 --- a/test/Microsoft.ML.IntegrationTests/Prediction.cs +++ b/test/Microsoft.ML.IntegrationTests/Prediction.cs @@ -97,5 +97,38 @@ public void ReconfigurablePredictionNoPipeline() Assert.True(pr.Score <= 0); } + [Fact] + public void PredictionEngineModelDisposal() + { + var mlContext = new MLContext(seed: 1); + var data = mlContext.Data.LoadFromEnumerable(TypeTestData.GenerateDataset()); + var pipeline = mlContext.BinaryClassification.Trainers.LbfgsLogisticRegression( + new Trainers.LbfgsLogisticRegressionBinaryTrainer.Options { NumberOfThreads = 1 }); + var model = pipeline.Fit(data); + + var engine = mlContext.Model.CreatePredictionEngine(model, new PredictionEngine.Options()); + + // Dispose of prediction engine, should dispose of model + engine.Dispose(); + + // Attempt to dispose of model but it should already be disposed. + Assert.Throws(() => model.Dispose()); + + // Make a new model/prediction engine. Set the options so prediction engine doesn't dispose + model = pipeline.Fit(data); + + var options = new PredictionEngine.Options() + { + OwnModelFile = false + }; + + engine = mlContext.Model.CreatePredictionEngine(model, options); + + // Dispose of prediction engine, shoudln't dispose of model + engine.Dispose(); + + // Manually dispose of model, no errors are thrown + model.Dispose(); + } } } From abb51654151b294620cb4fedec6e1f1b33ac41d8 Mon Sep 17 00:00:00 2001 From: Michael Sharp Date: Sat, 9 Oct 2021 04:00:17 -0700 Subject: [PATCH 3/6] Fixed Test --- test/Microsoft.ML.IntegrationTests/Prediction.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/Microsoft.ML.IntegrationTests/Prediction.cs b/test/Microsoft.ML.IntegrationTests/Prediction.cs index d59fb11e81..c67906b209 100644 --- a/test/Microsoft.ML.IntegrationTests/Prediction.cs +++ b/test/Microsoft.ML.IntegrationTests/Prediction.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Reflection; using Microsoft.ML.Calibrators; using Microsoft.ML.Data; using Microsoft.ML.IntegrationTests.Datasets; @@ -111,8 +112,12 @@ public void PredictionEngineModelDisposal() // Dispose of prediction engine, should dispose of model engine.Dispose(); - // Attempt to dispose of model but it should already be disposed. - Assert.Throws(() => model.Dispose()); + // Get disposed flag using reflection + var bfIsDisposed = BindingFlags.Instance | BindingFlags.NonPublic; + var field = model.GetType().BaseType.BaseType.GetField("_disposed", bfIsDisposed); + + // Make sure the model is actually disposed + Assert.True((bool)field.GetValue(model)); // Make a new model/prediction engine. Set the options so prediction engine doesn't dispose model = pipeline.Fit(data); @@ -127,7 +132,10 @@ public void PredictionEngineModelDisposal() // Dispose of prediction engine, shoudln't dispose of model engine.Dispose(); - // Manually dispose of model, no errors are thrown + // Make sure model is not disposed of. + Assert.False((bool)field.GetValue(model)); + + // Dispose of the model for test cleanliness model.Dispose(); } } From cc2158d9952b2c35adc361550af997005ec2df69 Mon Sep 17 00:00:00 2001 From: Michael Sharp Date: Mon, 11 Oct 2021 10:27:53 -0700 Subject: [PATCH 4/6] Updates from PR comments. --- .../Model/ModelOperationsCatalog.cs | 4 +- .../Model/PredictionEngineExtensions.cs | 6 +-- .../Prediction/PredictionEngine.cs | 52 +++++++++---------- .../PredictionEngine.cs | 8 +-- .../Prediction.cs | 8 +-- 5 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs index dd56b21fec..e2f65c6781 100644 --- a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs +++ b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs @@ -343,12 +343,12 @@ public PredictionEngine CreatePredictionEngine(ITransfor /// The class that defines the output data. /// The transformer to use for prediction. /// Advaned configuration options. - public PredictionEngine CreatePredictionEngine(ITransformer transformer, PredictionEngine.Options options) + public PredictionEngine CreatePredictionEngine(ITransformer transformer, PredictionEngineOptions options) where TSrc : class where TDst : class, new() { return transformer.CreatePredictionEngine(_env, options.IgnoreMissingColumns, - options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnModelFile); + options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnTransformer); } } } diff --git a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs index 1e6e67c1bb..0e43f07e88 100644 --- a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs +++ b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs @@ -24,11 +24,11 @@ internal static class PredictionEngineExtensions /// . /// Additional settings of the input schema. /// Additional settings of the output schema. - /// Whether the prediction engine owns the model file and should dispose of it. + /// Whether the prediction engine owns the transformer and should dispose of it. public static PredictionEngine CreatePredictionEngine(this ITransformer transformer, - IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownModelFile = true) + IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownTransformer = true) where TSrc : class where TDst : class, new() - => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition, ownModelFile); + => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition, ownTransformer); } } diff --git a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs index 79b5dfdc4d..eab051506d 100644 --- a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs +++ b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs @@ -59,8 +59,8 @@ public sealed class PredictionEngine : PredictionEngineBase : IDisposable private readonly DataViewConstructionUtils.InputRow _inputRow; private readonly IRowReadableAs _outputRow; private readonly Action _disposer; - private readonly bool _ownModelFile; + private readonly bool _ownTransformer; private bool _disposed; /// @@ -106,7 +106,7 @@ public abstract class PredictionEngineBase : IDisposable [BestFriend] private protected PredictionEngineBase(IHostEnvironment env, ITransformer transformer, bool ignoreMissingColumns, - SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownModelFile = true) + SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownTransformer = true) { Contracts.CheckValue(env, nameof(env)); env.AssertValue(transformer); @@ -114,7 +114,7 @@ private protected PredictionEngineBase(IHostEnvironment env, ITransformer transf var makeMapper = TransformerChecker(env, transformer); env.AssertValue(makeMapper); _inputRow = DataViewConstructionUtils.CreateInputRow(env, inputSchemaDefinition); - _ownModelFile = ownModelFile; + _ownTransformer = ownTransformer; PredictionEngineCore(env, _inputRow, makeMapper(_inputRow.Schema), ignoreMissingColumns, outputSchemaDefinition, out _disposer, out _outputRow); OutputSchema = Transformer.GetOutputSchema(_inputRow.Schema); } @@ -143,7 +143,7 @@ public void Dispose() _disposer?.Invoke(); - if (_ownModelFile) + if (_ownTransformer) (Transformer as IDisposable)?.Dispose(); _disposed = true; @@ -176,33 +176,29 @@ public TDst Predict(TSrc example) public abstract void Predict(TSrc example, ref TDst prediction); } - public sealed class PredictionEngine + /// + /// Options for the + /// + public sealed class PredictionEngineOptions { - /// - /// Options for the as used in - /// [RandomizedPca(Options)](xref:Microsoft.ML.PcaCatalog.RandomizedPca(Microsoft.ML.AnomalyDetectionCatalog.AnomalyDetectionTrainers,Microsoft.ML.Trainers.RandomizedPcaTrainer.Options)). - /// - public sealed class Options - { - [Argument(ArgumentType.AtMostOnce, HelpText = "Whether to throw an error if a column exists in the output schema but not the output object.", ShortName = "ignore", SortOrder = 50)] - public bool IgnoreMissingColumns = Defaults.IgnoreMissingColumns; + [Argument(ArgumentType.AtMostOnce, HelpText = "Whether to throw an error if a column exists in the output schema but not the output object.", ShortName = "ignore", SortOrder = 50)] + public bool IgnoreMissingColumns = Defaults.IgnoreMissingColumns; - [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the input schema.", ShortName = "input", SortOrder = 50)] - public SchemaDefinition InputSchemaDefinition = Defaults.InputSchemaDefinition; + [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the input schema.", ShortName = "input", SortOrder = 50)] + public SchemaDefinition InputSchemaDefinition = Defaults.InputSchemaDefinition; - [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the output schema.", ShortName = "output")] - public SchemaDefinition OutputSchemaDefinition = Defaults.OutputSchemaDefinition; + [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the output schema.", ShortName = "output")] + public SchemaDefinition OutputSchemaDefinition = Defaults.OutputSchemaDefinition; - [Argument(ArgumentType.AtMostOnce, HelpText = "Whether the prediction engine owns the model file and should dispose of it.", ShortName = "own")] - public bool OwnModelFile = Defaults.OwnModelFile; + [Argument(ArgumentType.AtMostOnce, HelpText = "Whether the prediction engine owns the model file and should dispose of it.", ShortName = "own")] + public bool OwnTransformer = Defaults.OwnTransformer; - internal static class Defaults - { - public const bool IgnoreMissingColumns = true; - public const SchemaDefinition InputSchemaDefinition = null; - public const SchemaDefinition OutputSchemaDefinition = null; - public const bool OwnModelFile = true; - } + internal static class Defaults + { + public const bool IgnoreMissingColumns = true; + public const SchemaDefinition InputSchemaDefinition = null; + public const SchemaDefinition OutputSchemaDefinition = null; + public const bool OwnTransformer = true; } } } diff --git a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs index 19d0cbb2cc..4c811c4fdf 100644 --- a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs +++ b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs @@ -152,8 +152,8 @@ public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer /// Contructor for creating time series specific prediction engine. It allows the time series model to be updated with the observations /// seen at prediction time via /// - public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer, PredictionEngine.Options options) : - base(env, CloneTransformers(transformer), options.IgnoreMissingColumns, options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnModelFile) + public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer, PredictionEngineOptions options) : + base(env, CloneTransformers(transformer), options.IgnoreMissingColumns, options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnTransformer) { } @@ -416,7 +416,7 @@ public static TimeSeriesPredictionEngine CreateTimeSeriesEngineClass describing the output schema of the prediction. /// The time series pipeline in the form of a . /// Usually - /// Advaned configuration options. + /// Advanced configuration options. ///

Example code can be found by searching for TimeSeriesPredictionEngine in ML.NET.

/// /// @@ -427,7 +427,7 @@ public static TimeSeriesPredictionEngine CreateTimeSeriesEngine /// public static TimeSeriesPredictionEngine CreateTimeSeriesEngine(this ITransformer transformer, IHostEnvironment env, - PredictionEngine.Options options) + PredictionEngineOptions options) where TSrc : class where TDst : class, new() { diff --git a/test/Microsoft.ML.IntegrationTests/Prediction.cs b/test/Microsoft.ML.IntegrationTests/Prediction.cs index c67906b209..6cb058df36 100644 --- a/test/Microsoft.ML.IntegrationTests/Prediction.cs +++ b/test/Microsoft.ML.IntegrationTests/Prediction.cs @@ -107,7 +107,7 @@ public void PredictionEngineModelDisposal() new Trainers.LbfgsLogisticRegressionBinaryTrainer.Options { NumberOfThreads = 1 }); var model = pipeline.Fit(data); - var engine = mlContext.Model.CreatePredictionEngine(model, new PredictionEngine.Options()); + var engine = mlContext.Model.CreatePredictionEngine(model, new PredictionEngineOptions()); // Dispose of prediction engine, should dispose of model engine.Dispose(); @@ -122,14 +122,14 @@ public void PredictionEngineModelDisposal() // Make a new model/prediction engine. Set the options so prediction engine doesn't dispose model = pipeline.Fit(data); - var options = new PredictionEngine.Options() + var options = new PredictionEngineOptions() { - OwnModelFile = false + OwnTransformer = false }; engine = mlContext.Model.CreatePredictionEngine(model, options); - // Dispose of prediction engine, shoudln't dispose of model + // Dispose of prediction engine, shouldn't dispose of model engine.Dispose(); // Make sure model is not disposed of. From 8a1dbdb02a87695456c449fe4152998b2a3e077b Mon Sep 17 00:00:00 2001 From: Michael Sharp Date: Mon, 11 Oct 2021 15:20:43 -0700 Subject: [PATCH 5/6] Comments from PR --- .../Model/ModelOperationsCatalog.cs | 2 +- .../Model/PredictionEngineExtensions.cs | 6 +++--- .../Prediction/PredictionEngine.cs | 18 +++++++++--------- .../PredictionEngine.cs | 4 ++-- .../Prediction.cs | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs index e2f65c6781..460019fb7f 100644 --- a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs +++ b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs @@ -348,7 +348,7 @@ public PredictionEngine CreatePredictionEngine(ITransfor where TDst : class, new() { return transformer.CreatePredictionEngine(_env, options.IgnoreMissingColumns, - options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnTransformer); + options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnsTransformer); } } } diff --git a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs index 0e43f07e88..2e162b764a 100644 --- a/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs +++ b/src/Microsoft.ML.Data/Model/PredictionEngineExtensions.cs @@ -24,11 +24,11 @@ internal static class PredictionEngineExtensions /// . /// Additional settings of the input schema. /// Additional settings of the output schema. - /// Whether the prediction engine owns the transformer and should dispose of it. + /// Whether the prediction engine owns the transformer and should dispose of it. public static PredictionEngine CreatePredictionEngine(this ITransformer transformer, - IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownTransformer = true) + IHostEnvironment env, bool ignoreMissingColumns = true, SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownsTransformer = true) where TSrc : class where TDst : class, new() - => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition, ownTransformer); + => new PredictionEngine(env, transformer, ignoreMissingColumns, inputSchemaDefinition, outputSchemaDefinition, ownsTransformer); } } diff --git a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs index eab051506d..8fdac4874b 100644 --- a/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs +++ b/src/Microsoft.ML.Data/Prediction/PredictionEngine.cs @@ -59,8 +59,8 @@ public sealed class PredictionEngine : PredictionEngineBase : IDisposable private readonly DataViewConstructionUtils.InputRow _inputRow; private readonly IRowReadableAs _outputRow; private readonly Action _disposer; - private readonly bool _ownTransformer; + private readonly bool _ownsTransformer; private bool _disposed; /// @@ -106,7 +106,7 @@ public abstract class PredictionEngineBase : IDisposable [BestFriend] private protected PredictionEngineBase(IHostEnvironment env, ITransformer transformer, bool ignoreMissingColumns, - SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownTransformer = true) + SchemaDefinition inputSchemaDefinition = null, SchemaDefinition outputSchemaDefinition = null, bool ownsTransformer = true) { Contracts.CheckValue(env, nameof(env)); env.AssertValue(transformer); @@ -114,7 +114,7 @@ private protected PredictionEngineBase(IHostEnvironment env, ITransformer transf var makeMapper = TransformerChecker(env, transformer); env.AssertValue(makeMapper); _inputRow = DataViewConstructionUtils.CreateInputRow(env, inputSchemaDefinition); - _ownTransformer = ownTransformer; + _ownsTransformer = ownsTransformer; PredictionEngineCore(env, _inputRow, makeMapper(_inputRow.Schema), ignoreMissingColumns, outputSchemaDefinition, out _disposer, out _outputRow); OutputSchema = Transformer.GetOutputSchema(_inputRow.Schema); } @@ -143,7 +143,7 @@ public void Dispose() _disposer?.Invoke(); - if (_ownTransformer) + if (_ownsTransformer) (Transformer as IDisposable)?.Dispose(); _disposed = true; @@ -190,15 +190,15 @@ public sealed class PredictionEngineOptions [Argument(ArgumentType.AtMostOnce, HelpText = "Additional settings of the output schema.", ShortName = "output")] public SchemaDefinition OutputSchemaDefinition = Defaults.OutputSchemaDefinition; - [Argument(ArgumentType.AtMostOnce, HelpText = "Whether the prediction engine owns the model file and should dispose of it.", ShortName = "own")] - public bool OwnTransformer = Defaults.OwnTransformer; + [Argument(ArgumentType.AtMostOnce, HelpText = "Whether the prediction engine owns the transformer and should dispose of it.", ShortName = "own")] + public bool OwnsTransformer = Defaults.OwnsTransformer; internal static class Defaults { public const bool IgnoreMissingColumns = true; public const SchemaDefinition InputSchemaDefinition = null; public const SchemaDefinition OutputSchemaDefinition = null; - public const bool OwnTransformer = true; + public const bool OwnsTransformer = true; } } } diff --git a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs index 4c811c4fdf..d1742dc941 100644 --- a/src/Microsoft.ML.TimeSeries/PredictionEngine.cs +++ b/src/Microsoft.ML.TimeSeries/PredictionEngine.cs @@ -152,8 +152,8 @@ public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer /// Contructor for creating time series specific prediction engine. It allows the time series model to be updated with the observations /// seen at prediction time via /// - public TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer, PredictionEngineOptions options) : - base(env, CloneTransformers(transformer), options.IgnoreMissingColumns, options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnTransformer) + internal TimeSeriesPredictionEngine(IHostEnvironment env, ITransformer transformer, PredictionEngineOptions options) : + base(env, CloneTransformers(transformer), options.IgnoreMissingColumns, options.InputSchemaDefinition, options.OutputSchemaDefinition, options.OwnsTransformer) { } diff --git a/test/Microsoft.ML.IntegrationTests/Prediction.cs b/test/Microsoft.ML.IntegrationTests/Prediction.cs index 6cb058df36..ec5c333561 100644 --- a/test/Microsoft.ML.IntegrationTests/Prediction.cs +++ b/test/Microsoft.ML.IntegrationTests/Prediction.cs @@ -124,7 +124,7 @@ public void PredictionEngineModelDisposal() var options = new PredictionEngineOptions() { - OwnTransformer = false + OwnsTransformer = false }; engine = mlContext.Model.CreatePredictionEngine(model, options); From 218983947e14215c317fe5e14abc8f81066261aa Mon Sep 17 00:00:00 2001 From: Michael Sharp <51342856+michaelgsharp@users.noreply.github.com> Date: Mon, 11 Oct 2021 15:52:20 -0700 Subject: [PATCH 6/6] Update src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs Co-authored-by: Eric Erhardt --- src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs index 460019fb7f..b9f9f4bb80 100644 --- a/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs +++ b/src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs @@ -342,7 +342,7 @@ public PredictionEngine CreatePredictionEngine(ITransfor /// The class that defines the input data. /// The class that defines the output data. /// The transformer to use for prediction. - /// Advaned configuration options. + /// Advanced configuration options. public PredictionEngine CreatePredictionEngine(ITransformer transformer, PredictionEngineOptions options) where TSrc : class where TDst : class, new()