From 1a826dca914b308de0e50eaedcabdcf9525bcf3c Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 13 Feb 2020 18:10:27 -0800 Subject: [PATCH 01/14] Experimentations --- .../SaveOnnxCommand.cs | 32 +++++++++ test/Microsoft.ML.Tests/OnnxConversionTest.cs | 70 ++++++++++++------- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 966fe255f4..a4fe669cf0 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -13,6 +13,7 @@ using Microsoft.ML.Internal.Utilities; using Microsoft.ML.Model.OnnxConverter; using Microsoft.ML.Runtime; +using Microsoft.ML.Transforms; using Newtonsoft.Json; using static Microsoft.ML.Model.OnnxConverter.OnnxCSharpToProtoWrapper; @@ -209,6 +210,31 @@ internal static ModelProto ConvertTransformListToOnnxModel(OnnxContextImpl ctx, return ctx.MakeModel(); } + private IDataView MyTest(PredictorModel predictorModel, IDataView inputData) + { + var host = new MLContext(); + RoleMappedData data; + IPredictor predictor; + predictorModel.PrepareData(host, inputData, out data, out predictor); + + IDataView scoredData; + //using (var ch = host.Start("Creating scoring pipeline")) + { + //ch.Trace("Creating pipeline"); + var bindable = ScoreUtils.GetSchemaBindableMapper(host, predictor); + //ch.AssertValue(bindable); + + var mapper = bindable.Bind(host, data.Schema); + var scorer = ScoreUtils.GetScorerComponent(host, mapper, null); + scoredData = scorer.CreateComponent(host, data.Data, mapper, predictorModel.GetTrainingSchema(host)); + } + + var xf = new KeyToValueMappingTransformer(host, "PredictedLabel").Transform(scoredData); + var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(host, xf, scoredData), OutputData = xf }; + var outputDataView = output.OutputData; + return outputDataView; + } + private void Run(IChannel ch) { ILegacyDataLoader loader = null; @@ -240,6 +266,7 @@ private void Run(IChannel ch) view = _predictiveModel.TransformModel.Apply(Host, new EmptyDataView(Host, _predictiveModel.TransformModel.InputSchema)); rawPred = _predictiveModel.Predictor; trainSchema = _predictiveModel.GetTrainingSchema(Host); + // view = MyTest(_predictiveModel, view); } // Create the ONNX context for storing global information @@ -276,6 +303,11 @@ private void Run(IChannel ch) Host.Assert(scorePipe.Source == end); end = scorePipe; transforms.AddLast(scoreOnnx); + + var xf = new KeyToValueMappingTransformer(Host, "PredictedLabel").Transform(scorePipe); + var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, scorePipe), OutputData = xf }; + end = output.OutputData; + transforms.AddLast(end as ITransformCanSaveOnnx); } else { diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index 52cd84c8f8..bd236d088e 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -878,6 +878,50 @@ public void LoadingPredictorModelAndOnnxConversionTest() Done(); } + [Fact] + public void TestCustomOnnxGraph() + { + var modelPath = @"C:\Users\anvelazq\Desktop\is23\generated_model.zip"; + var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23\onnx-json.json"; + var onnxPath = @"C:\Users\anvelazq\Desktop\is23\onnx-model.onnx"; + + // Make entry point graph to conduct ONNX conversion. + var inputGraph = string.Format(@" + {{ + 'Inputs': {{ + 'model': '{0}' + }}, + 'Nodes': [ + {{ + 'Inputs': {{ + 'Domain': 'com.microsoft.models', + 'Json': '{1}', + 'PredictiveModel': '$model', + 'Onnx': '{2}', + 'OnnxVersion': 'Experimental' + }}, + 'Name': 'Models.OnnxConverter', + 'Outputs': {{}} + }} + ], + 'Outputs': {{}} + }} + ", modelPath.Replace("\\", "\\\\"), onnxJsonPath.Replace("\\", "\\\\"), onnxPath.Replace("\\", "\\\\")); + + // Write entry point graph for ONNX conversion into file so that it can be invoke by graph runner below. + var jsonPath = DeleteOutputPath("graph.json"); + File.WriteAllLines(jsonPath, new[] { inputGraph }); + + // Onnx converter's assembly is not loaded by default, so we need to register it before calling it. + Env.ComponentCatalog.RegisterAssembly(typeof(OnnxExportExtensions).Assembly); + + // Execute the saved entry point graph to convert the saved model to ONNX format. + var args = new ExecuteGraphCommand.Arguments() { GraphPath = jsonPath }; + var cmd = new ExecuteGraphCommand(Env, args); + cmd.Run(); + + Done(); + } [Fact] public void RemoveVariablesInPipelineTest() @@ -1413,35 +1457,9 @@ public void MulticlassTrainersOnnxConversionTest() List> estimators = new List>() { - mlContext.MulticlassClassification.Trainers.LbfgsMaximumEntropy(), - mlContext.MulticlassClassification.Trainers.NaiveBayes(), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.AveragedPerceptron()), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.AveragedPerceptron(), useProbabilities:false), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.LbfgsLogisticRegression()), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.LbfgsLogisticRegression(), useProbabilities:false), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.LinearSvm()), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.LinearSvm(), useProbabilities:false), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.FastForest()), - mlContext.MulticlassClassification.Trainers.OneVersusAll( - mlContext.BinaryClassification.Trainers.FastForest(), useProbabilities:false), mlContext.MulticlassClassification.Trainers.SdcaMaximumEntropy(), - mlContext.MulticlassClassification.Trainers.SdcaNonCalibrated() }; - if (Environment.Is64BitProcess) - { - estimators.Add(mlContext.MulticlassClassification.Trainers.LightGbm()); - estimators.Add(mlContext.MulticlassClassification.Trainers.LightGbm( - new LightGbmMulticlassTrainer.Options { UseSoftmax = true })); - } - var initialPipeline = mlContext.Transforms.ReplaceMissingValues("Features") .Append(mlContext.Transforms.NormalizeMinMax("Features")) .Append(mlContext.Transforms.Conversion.MapValueToKey("Label")); From 009a84fbda7b6d517d743b12108e49264fe0e7dc Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 13 Feb 2020 18:26:55 -0800 Subject: [PATCH 02/14] Removing MyTest method --- .../SaveOnnxCommand.cs | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index a4fe669cf0..38b9fd044d 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -210,31 +210,6 @@ internal static ModelProto ConvertTransformListToOnnxModel(OnnxContextImpl ctx, return ctx.MakeModel(); } - private IDataView MyTest(PredictorModel predictorModel, IDataView inputData) - { - var host = new MLContext(); - RoleMappedData data; - IPredictor predictor; - predictorModel.PrepareData(host, inputData, out data, out predictor); - - IDataView scoredData; - //using (var ch = host.Start("Creating scoring pipeline")) - { - //ch.Trace("Creating pipeline"); - var bindable = ScoreUtils.GetSchemaBindableMapper(host, predictor); - //ch.AssertValue(bindable); - - var mapper = bindable.Bind(host, data.Schema); - var scorer = ScoreUtils.GetScorerComponent(host, mapper, null); - scoredData = scorer.CreateComponent(host, data.Data, mapper, predictorModel.GetTrainingSchema(host)); - } - - var xf = new KeyToValueMappingTransformer(host, "PredictedLabel").Transform(scoredData); - var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(host, xf, scoredData), OutputData = xf }; - var outputDataView = output.OutputData; - return outputDataView; - } - private void Run(IChannel ch) { ILegacyDataLoader loader = null; From f11c197ae461e822e611bcd3f3fa1b2f7b203621 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 13 Feb 2020 18:27:20 -0800 Subject: [PATCH 03/14] Removing unused comment --- src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 38b9fd044d..1fe2349a29 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -241,7 +241,6 @@ private void Run(IChannel ch) view = _predictiveModel.TransformModel.Apply(Host, new EmptyDataView(Host, _predictiveModel.TransformModel.InputSchema)); rawPred = _predictiveModel.Predictor; trainSchema = _predictiveModel.GetTrainingSchema(Host); - // view = MyTest(_predictiveModel, view); } // Create the ONNX context for storing global information From 3a972f5fc658a5e4037fd50491ee2cd4fc3a4c16 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 13 Feb 2020 18:30:20 -0800 Subject: [PATCH 04/14] Fixing a change a made by mistake in an unrelated test --- test/Microsoft.ML.Tests/OnnxConversionTest.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index bd236d088e..d797608eab 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -1457,9 +1457,35 @@ public void MulticlassTrainersOnnxConversionTest() List> estimators = new List>() { + mlContext.MulticlassClassification.Trainers.LbfgsMaximumEntropy(), + mlContext.MulticlassClassification.Trainers.NaiveBayes(), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.AveragedPerceptron()), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.AveragedPerceptron(), useProbabilities:false), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.LbfgsLogisticRegression()), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.LbfgsLogisticRegression(), useProbabilities:false), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.LinearSvm()), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.LinearSvm(), useProbabilities:false), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.FastForest()), + mlContext.MulticlassClassification.Trainers.OneVersusAll( + mlContext.BinaryClassification.Trainers.FastForest(), useProbabilities:false), mlContext.MulticlassClassification.Trainers.SdcaMaximumEntropy(), + mlContext.MulticlassClassification.Trainers.SdcaNonCalibrated() }; + if (Environment.Is64BitProcess) + { + estimators.Add(mlContext.MulticlassClassification.Trainers.LightGbm()); + estimators.Add(mlContext.MulticlassClassification.Trainers.LightGbm( + new LightGbmMulticlassTrainer.Options { UseSoftmax = true })); + } + var initialPipeline = mlContext.Transforms.ReplaceMissingValues("Features") .Append(mlContext.Transforms.NormalizeMinMax("Features")) .Append(mlContext.Transforms.Conversion.MapValueToKey("Label")); From 3e432a17f3a9673c0e7f3f9adf90a7133671d75e Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Fri, 14 Feb 2020 17:29:36 -0800 Subject: [PATCH 05/14] Added test for debugging purposes --- test/Microsoft.ML.Tests/OnnxConversionTest.cs | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index d797608eab..e10f994152 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -878,12 +878,13 @@ public void LoadingPredictorModelAndOnnxConversionTest() Done(); } + //MYTODO: Remove this once I am finished with this PR [Fact] - public void TestCustomOnnxGraph() + public void TestOnnxCustomPredictorGraph() { - var modelPath = @"C:\Users\anvelazq\Desktop\is23\generated_model.zip"; - var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23\onnx-json.json"; - var onnxPath = @"C:\Users\anvelazq\Desktop\is23\onnx-model.onnx"; + var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; + var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.json"; + var onnxPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.onnx"; // Make entry point graph to conduct ONNX conversion. var inputGraph = string.Format(@" @@ -923,6 +924,52 @@ public void TestCustomOnnxGraph() Done(); } + //MYTODO: Remove this once I am finished with this PR + [Fact] + public void TestOnnxCustomTransformGraph() + { + var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; + var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.json"; + var onnxPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.onnx"; + + // Make entry point graph to conduct ONNX conversion. + var inputGraph = string.Format(@" + {{ + 'Inputs': {{ + 'model': '{0}' + }}, + 'Nodes': [ + {{ + 'Inputs': {{ + 'Domain': 'com.microsoft.models', + 'Json': '{1}', + 'Model': '$model', + 'Onnx': '{2}', + 'OnnxVersion': 'Experimental' + }}, + 'Name': 'Models.OnnxConverter', + 'Outputs': {{}} + }} + ], + 'Outputs': {{}} + }} + ", modelPath.Replace("\\", "\\\\"), onnxJsonPath.Replace("\\", "\\\\"), onnxPath.Replace("\\", "\\\\")); + + // Write entry point graph for ONNX conversion into file so that it can be invoke by graph runner below. + var jsonPath = DeleteOutputPath("graph.json"); + File.WriteAllLines(jsonPath, new[] { inputGraph }); + + // Onnx converter's assembly is not loaded by default, so we need to register it before calling it. + Env.ComponentCatalog.RegisterAssembly(typeof(OnnxExportExtensions).Assembly); + + // Execute the saved entry point graph to convert the saved model to ONNX format. + var args = new ExecuteGraphCommand.Arguments() { GraphPath = jsonPath }; + var cmd = new ExecuteGraphCommand(Env, args); + cmd.Run(); + + Done(); + } + [Fact] public void RemoveVariablesInPipelineTest() { From 11b84d06d3650486d16f0c7af8efeb05ac1e8d25 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Fri, 14 Feb 2020 17:30:53 -0800 Subject: [PATCH 06/14] Add support to map back keys from columns that where untouched by the model --- .../SaveOnnxCommand.cs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 1fe2349a29..291ec0fc15 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -279,7 +279,7 @@ private void Run(IChannel ch) transforms.AddLast(scoreOnnx); var xf = new KeyToValueMappingTransformer(Host, "PredictedLabel").Transform(scorePipe); - var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, scorePipe), OutputData = xf }; + var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, scorePipe), OutputData = xf }; //MYTODO: Is this step necessary? I'd think it isn't end = output.OutputData; transforms.AddLast(end as ITransformCanSaveOnnx); } @@ -296,6 +296,29 @@ private void Run(IChannel ch) nameof(Arguments.LoadPredictor), "We were explicitly told to load the predictor but one was not present."); } + // Convert back to values the KeyDataViewType columns that appear both in input and output (i.e those that remained untouched + // by the model. + //MYTODO: perhaps move this into another function. + var outputNames = new HashSet(); + foreach (var col in end.Schema) + if(col.Type is KeyDataViewType) + outputNames.Add(col.Name); + + var inputNames = new HashSet(); + foreach (var col in source.Schema) + if(col.Type is KeyDataViewType && col.IsHidden == false) + inputNames.Add(col.Name); + + outputNames.IntersectWith(inputNames); + + foreach(var name in outputNames) + { + var xf = new KeyToValueMappingTransformer(Host, name).Transform(end); + var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, end), OutputData = xf }; // MYTODO: is this step necessary? I'd think it isn't + end = output.OutputData; + transforms.AddLast(end as ITransformCanSaveOnnx); + } + var model = ConvertTransformListToOnnxModel(ctx, ch, source, end, transforms, _inputsToDrop, _outputsToDrop); using (var file = Host.CreateOutputFile(_outputModelPath)) From a786d9f5f6e66d94b8e4e8573addb639da17851b Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Tue, 18 Feb 2020 19:05:22 -0800 Subject: [PATCH 07/14] Added security checks and removed redundant steps --- .../SaveOnnxCommand.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 291ec0fc15..5f8da5da75 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -278,10 +278,15 @@ private void Run(IChannel ch) end = scorePipe; transforms.AddLast(scoreOnnx); - var xf = new KeyToValueMappingTransformer(Host, "PredictedLabel").Transform(scorePipe); - var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, scorePipe), OutputData = xf }; //MYTODO: Is this step necessary? I'd think it isn't - end = output.OutputData; - transforms.AddLast(end as ITransformCanSaveOnnx); + if(rawPred.PredictionKind == PredictionKind.BinaryClassification || rawPred.PredictionKind == PredictionKind.MulticlassClassification) + { + if(scorePipe.Schema.GetColumnOrNull("PredictedLabel")?.Type is KeyDataViewType) + { + var outputData = new KeyToValueMappingTransformer(Host, "PredictedLabel").Transform(scorePipe); + end = outputData; + transforms.AddLast(outputData as ITransformCanSaveOnnx); + } + } } else { @@ -299,9 +304,10 @@ private void Run(IChannel ch) // Convert back to values the KeyDataViewType columns that appear both in input and output (i.e those that remained untouched // by the model. //MYTODO: perhaps move this into another function. + //MYTODO: Filter the following with _inputsToDrop and _outputsToDrop? var outputNames = new HashSet(); foreach (var col in end.Schema) - if(col.Type is KeyDataViewType) + if(col.Type is KeyDataViewType && col.IsHidden == false) outputNames.Add(col.Name); var inputNames = new HashSet(); @@ -313,9 +319,8 @@ private void Run(IChannel ch) foreach(var name in outputNames) { - var xf = new KeyToValueMappingTransformer(Host, name).Transform(end); - var output = new CommonOutputs.TransformOutput { Model = new TransformModelImpl(Host, xf, end), OutputData = xf }; // MYTODO: is this step necessary? I'd think it isn't - end = output.OutputData; + var outputData = new KeyToValueMappingTransformer(Host, name).Transform(end); + end = outputData; transforms.AddLast(end as ITransformCanSaveOnnx); } From cfa264d070647ee58d12f11dd996ce9cf5173458 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 20 Feb 2020 16:53:38 -0800 Subject: [PATCH 08/14] Added todo comment and formatting --- src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 5f8da5da75..12886bed49 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -301,24 +301,23 @@ private void Run(IChannel ch) nameof(Arguments.LoadPredictor), "We were explicitly told to load the predictor but one was not present."); } - // Convert back to values the KeyDataViewType columns that appear both in input and output (i.e those that remained untouched - // by the model. - //MYTODO: perhaps move this into another function. - //MYTODO: Filter the following with _inputsToDrop and _outputsToDrop? + // Convert back to values the KeyDataViewType columns that appear both in input and output + // (i.e those that remained untouched by the model). var outputNames = new HashSet(); foreach (var col in end.Schema) - if(col.Type is KeyDataViewType && col.IsHidden == false) + if (col.Type is KeyDataViewType && col.IsHidden == false) outputNames.Add(col.Name); var inputNames = new HashSet(); foreach (var col in source.Schema) - if(col.Type is KeyDataViewType && col.IsHidden == false) + if (col.Type is KeyDataViewType && col.IsHidden == false) inputNames.Add(col.Name); outputNames.IntersectWith(inputNames); - foreach(var name in outputNames) + foreach (var name in outputNames) { + // MYTODO: Add in here any check necessary to see if the column actually has KeyValue Annotations var outputData = new KeyToValueMappingTransformer(Host, name).Transform(end); end = outputData; transforms.AddLast(end as ITransformCanSaveOnnx); From 86fb14a7109aac3f4e93bc5b7a748a1cadb4dd6b Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 20 Feb 2020 16:56:27 -0800 Subject: [PATCH 09/14] Skip Custom tests on CI --- test/Microsoft.ML.Tests/OnnxConversionTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index e10f994152..0607853ab5 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -880,6 +880,7 @@ public void LoadingPredictorModelAndOnnxConversionTest() //MYTODO: Remove this once I am finished with this PR [Fact] + [Trait("Category", "SkipInCI")] public void TestOnnxCustomPredictorGraph() { var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; @@ -926,6 +927,7 @@ public void TestOnnxCustomPredictorGraph() //MYTODO: Remove this once I am finished with this PR [Fact] + [Trait("Category", "SkipInCI")] public void TestOnnxCustomTransformGraph() { var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; From 3e5b2bd78a5057e99c819022ad000d8723fc8bce Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Thu, 20 Feb 2020 17:16:36 -0800 Subject: [PATCH 10/14] Removed my custom tests --- test/Microsoft.ML.Tests/OnnxConversionTest.cs | 94 ------------------- 1 file changed, 94 deletions(-) diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index 529e92795d..65ac85bd12 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -864,100 +864,6 @@ public void LoadingPredictorModelAndOnnxConversionTest() Done(); } - //MYTODO: Remove this once I am finished with this PR - [Fact] - [Trait("Category", "SkipInCI")] - public void TestOnnxCustomPredictorGraph() - { - var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; - var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.json"; - var onnxPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.onnx"; - - // Make entry point graph to conduct ONNX conversion. - var inputGraph = string.Format(@" - {{ - 'Inputs': {{ - 'model': '{0}' - }}, - 'Nodes': [ - {{ - 'Inputs': {{ - 'Domain': 'com.microsoft.models', - 'Json': '{1}', - 'PredictiveModel': '$model', - 'Onnx': '{2}', - 'OnnxVersion': 'Experimental' - }}, - 'Name': 'Models.OnnxConverter', - 'Outputs': {{}} - }} - ], - 'Outputs': {{}} - }} - ", modelPath.Replace("\\", "\\\\"), onnxJsonPath.Replace("\\", "\\\\"), onnxPath.Replace("\\", "\\\\")); - - // Write entry point graph for ONNX conversion into file so that it can be invoke by graph runner below. - var jsonPath = DeleteOutputPath("graph.json"); - File.WriteAllLines(jsonPath, new[] { inputGraph }); - - // Onnx converter's assembly is not loaded by default, so we need to register it before calling it. - Env.ComponentCatalog.RegisterAssembly(typeof(OnnxExportExtensions).Assembly); - - // Execute the saved entry point graph to convert the saved model to ONNX format. - var args = new ExecuteGraphCommand.Arguments() { GraphPath = jsonPath }; - var cmd = new ExecuteGraphCommand(Env, args); - cmd.Run(); - - Done(); - } - - //MYTODO: Remove this once I am finished with this PR - [Fact] - [Trait("Category", "SkipInCI")] - public void TestOnnxCustomTransformGraph() - { - var modelPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.zip"; - var onnxJsonPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.json"; - var onnxPath = @"C:\Users\anvelazq\Desktop\is23repros\Handler.onnx"; - - // Make entry point graph to conduct ONNX conversion. - var inputGraph = string.Format(@" - {{ - 'Inputs': {{ - 'model': '{0}' - }}, - 'Nodes': [ - {{ - 'Inputs': {{ - 'Domain': 'com.microsoft.models', - 'Json': '{1}', - 'Model': '$model', - 'Onnx': '{2}', - 'OnnxVersion': 'Experimental' - }}, - 'Name': 'Models.OnnxConverter', - 'Outputs': {{}} - }} - ], - 'Outputs': {{}} - }} - ", modelPath.Replace("\\", "\\\\"), onnxJsonPath.Replace("\\", "\\\\"), onnxPath.Replace("\\", "\\\\")); - - // Write entry point graph for ONNX conversion into file so that it can be invoke by graph runner below. - var jsonPath = DeleteOutputPath("graph.json"); - File.WriteAllLines(jsonPath, new[] { inputGraph }); - - // Onnx converter's assembly is not loaded by default, so we need to register it before calling it. - Env.ComponentCatalog.RegisterAssembly(typeof(OnnxExportExtensions).Assembly); - - // Execute the saved entry point graph to convert the saved model to ONNX format. - var args = new ExecuteGraphCommand.Arguments() { GraphPath = jsonPath }; - var cmd = new ExecuteGraphCommand(Env, args); - cmd.Run(); - - Done(); - } - [Fact] public void RemoveVariablesInPipelineTest() { From 8864d8ecfdb1b4258ce3940a64566f86fd63f2cb Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Fri, 21 Feb 2020 18:28:27 -0800 Subject: [PATCH 11/14] Added security checks to confirm that the PredictedLabel column has KeyValues annotations and added explanatory comments --- .../SaveOnnxCommand.cs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 3f671ace2e..52b7ee6236 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -312,11 +312,19 @@ private void Run(IChannel ch) if(rawPred.PredictionKind == PredictionKind.BinaryClassification || rawPred.PredictionKind == PredictionKind.MulticlassClassification) { - if(scorePipe.Schema.GetColumnOrNull("PredictedLabel")?.Type is KeyDataViewType) + // Check if the PredictedLabel Column is a KeyDataViewType and has KeyValue Annotations. + // If it does, add a KeyToValueMappingTransformer, to enable NimbusML to get the values back + // when using an ONNX model, as described in https://github.com/dotnet/machinelearning/pull/4841 + var predictedLabelColumn = scorePipe.Schema.GetColumnOrNull(DefaultColumnNames.PredictedLabel); + if (predictedLabelColumn?.Type.GetItemType() is KeyDataViewType keyType) { - var outputData = new KeyToValueMappingTransformer(Host, "PredictedLabel").Transform(scorePipe); - end = outputData; - transforms.AddLast(outputData as ITransformCanSaveOnnx); + var metaColumn = predictedLabelColumn?.Annotations.Schema.GetColumnOrNull(AnnotationUtils.Kinds.KeyValues); + if(metaColumn != null && metaColumn.Value.Type is VectorDataViewType vectorType && keyType.Count == (ulong)vectorType.Size) + { + var outputData = new KeyToValueMappingTransformer(Host, DefaultColumnNames.PredictedLabel).Transform(scorePipe); + end = outputData; + transforms.AddLast(outputData as ITransformCanSaveOnnx); + } } } } @@ -334,7 +342,9 @@ private void Run(IChannel ch) } // Convert back to values the KeyDataViewType columns that appear both in input and output - // (i.e those that remained untouched by the model). + // (i.e those that remained untouched by the model). This is done to enable NimbusML to get these values + // as described in https://github.com/dotnet/machinelearning/pull/4841 + var outputNames = new HashSet(); foreach (var col in end.Schema) if (col.Type is KeyDataViewType && col.IsHidden == false) From ff9650e4472b6cfae481d0a75742d1526c736947 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 24 Feb 2020 15:14:19 -0800 Subject: [PATCH 12/14] Added HasKeyValues helper method and changed pass-through columns filtering --- .../SaveOnnxCommand.cs | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index 2b9e8fc568..d6fa709d5f 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -242,6 +242,21 @@ private static void AddSlotNames(OnnxContextImpl ctx, DataViewSchema.Column colu ctx.AddOutputVariable(NumberDataViewType.Int64, labelEncoderOutput); } + // Checks if a column has KeyValues Annotations of any type, + // So to know if it is safe to use KeyToValue Transformer on it. + private bool HasKeyValues(DataViewSchema.Column column) + { + if (column.Type.GetItemType() is KeyDataViewType keyType) + { + var metaColumn = column.Annotations.Schema.GetColumnOrNull(AnnotationUtils.Kinds.KeyValues); + return metaColumn != null && + metaColumn.Value.Type is VectorDataViewType vectorType && + keyType.Count == (ulong)vectorType.Size; + } + + return false; + } + private void Run(IChannel ch) { ILegacyDataLoader loader = null; @@ -316,15 +331,11 @@ private void Run(IChannel ch) // If it does, add a KeyToValueMappingTransformer, to enable NimbusML to get the values back // when using an ONNX model, as described in https://github.com/dotnet/machinelearning/pull/4841 var predictedLabelColumn = scorePipe.Schema.GetColumnOrNull(DefaultColumnNames.PredictedLabel); - if (predictedLabelColumn?.Type.GetItemType() is KeyDataViewType keyType) + if (predictedLabelColumn.HasValue && HasKeyValues(predictedLabelColumn.Value)) { - var metaColumn = predictedLabelColumn?.Annotations.Schema.GetColumnOrNull(AnnotationUtils.Kinds.KeyValues); - if(metaColumn != null && metaColumn.Value.Type is VectorDataViewType vectorType && keyType.Count == (ulong)vectorType.Size) - { - var outputData = new KeyToValueMappingTransformer(Host, DefaultColumnNames.PredictedLabel).Transform(scorePipe); - end = outputData; - transforms.AddLast(outputData as ITransformCanSaveOnnx); - } + var outputData = new KeyToValueMappingTransformer(Host, DefaultColumnNames.PredictedLabel).Transform(scorePipe); + end = outputData; + transforms.AddLast(outputData as ITransformCanSaveOnnx); } } } @@ -341,25 +352,25 @@ private void Run(IChannel ch) nameof(Arguments.LoadPredictor), "We were explicitly told to load the predictor but one was not present."); } - // Convert back to values the KeyDataViewType columns that appear both in input and output + // Convert back to values the KeyDataViewType "pass-through" columns // (i.e those that remained untouched by the model). This is done to enable NimbusML to get these values // as described in https://github.com/dotnet/machinelearning/pull/4841 - var outputNames = new HashSet(); - foreach (var col in end.Schema) - if (col.Type is KeyDataViewType && col.IsHidden == false) - outputNames.Add(col.Name); - - var inputNames = new HashSet(); + var inputKeyDataViewTypeColumnsNames = new HashSet(); foreach (var col in source.Schema) - if (col.Type is KeyDataViewType && col.IsHidden == false) - inputNames.Add(col.Name); + if (col.IsHidden == false && HasKeyValues(col)) + inputKeyDataViewTypeColumnsNames.Add(col.Name); + + var passThroughColumnNames = new HashSet(); + var onlyDistinctColumnNames = end.Schema.Select(c => c.Name).Distinct(); // only check column names that appear once in the output schema + foreach (var col in end.Schema) + if (col.IsHidden == false && onlyDistinctColumnNames.Contains(col.Name) && HasKeyValues(col)) + passThroughColumnNames.Add(col.Name); - outputNames.IntersectWith(inputNames); + passThroughColumnNames.IntersectWith(inputKeyDataViewTypeColumnsNames); // Only count those columns that were in the input of the pipeline - foreach (var name in outputNames) + foreach (var name in passThroughColumnNames) { - // MYTODO: Add in here any check necessary to see if the column actually has KeyValue Annotations var outputData = new KeyToValueMappingTransformer(Host, name).Transform(end); end = outputData; transforms.AddLast(end as ITransformCanSaveOnnx); From 953ad7b168aaa7c4a1f26f12196bab6f290870ae Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 24 Feb 2020 15:15:53 -0800 Subject: [PATCH 13/14] Fix extra space --- test/Microsoft.ML.Tests/OnnxConversionTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Microsoft.ML.Tests/OnnxConversionTest.cs b/test/Microsoft.ML.Tests/OnnxConversionTest.cs index 0f8caa9295..ad2bbd0a20 100644 --- a/test/Microsoft.ML.Tests/OnnxConversionTest.cs +++ b/test/Microsoft.ML.Tests/OnnxConversionTest.cs @@ -878,6 +878,7 @@ public void LoadingPredictorModelAndOnnxConversionTest() Done(); } + [Fact] public void RemoveVariablesInPipelineTest() { From d70843ffcff8be8f99b83b39a780bc9b306e741b Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 24 Feb 2020 16:06:31 -0800 Subject: [PATCH 14/14] Corrected and refactored GetPassThroughKeyDataViewTypeColumnsNames logic --- .../SaveOnnxCommand.cs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs index d6fa709d5f..bde5bcd373 100644 --- a/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs +++ b/src/Microsoft.ML.OnnxConverter/SaveOnnxCommand.cs @@ -257,6 +257,37 @@ metaColumn.Value.Type is VectorDataViewType vectorType && return false; } + // Get the names of the KeyDataViewType columns that aren't affected by the pipeline that is being exported to ONNX. + private HashSet GetPassThroughKeyDataViewTypeColumnsNames(IDataView source, IDataView end) + { + var inputKeyDataViewTypeColumnsNames = new HashSet(); + foreach (var col in source.Schema) + if (col.IsHidden == false && HasKeyValues(col)) + inputKeyDataViewTypeColumnsNames.Add(col.Name); + + var passThroughColumnNames = new HashSet(); + var outputColumnNames = new HashSet(); + foreach (var col in end.Schema) + { + if (outputColumnNames.Contains(col.Name)) + { + // "Pass through" column names appear only once in the output schema + passThroughColumnNames.Remove(col.Name); + } + else + { + // We are only interested in the KeyDataViewType outpus columns + if (col.IsHidden == false && HasKeyValues(col)) + passThroughColumnNames.Add(col.Name); + } + outputColumnNames.Add(col.Name); + } + + // Only count those columns that were in the input of the pipeline + passThroughColumnNames.IntersectWith(inputKeyDataViewTypeColumnsNames); + return passThroughColumnNames; + } + private void Run(IChannel ch) { ILegacyDataLoader loader = null; @@ -356,19 +387,7 @@ private void Run(IChannel ch) // (i.e those that remained untouched by the model). This is done to enable NimbusML to get these values // as described in https://github.com/dotnet/machinelearning/pull/4841 - var inputKeyDataViewTypeColumnsNames = new HashSet(); - foreach (var col in source.Schema) - if (col.IsHidden == false && HasKeyValues(col)) - inputKeyDataViewTypeColumnsNames.Add(col.Name); - - var passThroughColumnNames = new HashSet(); - var onlyDistinctColumnNames = end.Schema.Select(c => c.Name).Distinct(); // only check column names that appear once in the output schema - foreach (var col in end.Schema) - if (col.IsHidden == false && onlyDistinctColumnNames.Contains(col.Name) && HasKeyValues(col)) - passThroughColumnNames.Add(col.Name); - - passThroughColumnNames.IntersectWith(inputKeyDataViewTypeColumnsNames); // Only count those columns that were in the input of the pipeline - + var passThroughColumnNames = GetPassThroughKeyDataViewTypeColumnsNames(source, end); foreach (var name in passThroughColumnNames) { var outputData = new KeyToValueMappingTransformer(Host, name).Transform(end);