From d8e1dfc4b5a205e8bff973c8883dbe9bbe559188 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Wed, 6 Nov 2019 13:57:25 -0800 Subject: [PATCH 01/14] Loading both create and constructor method and log when both of them have the same visibility. --- .../ComponentModel/ComponentCatalog.cs | 77 +++++++++++++++++-- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index b8d282a5a3..55d2ccee3c 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -414,21 +414,82 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp ctor = null; create = null; requireEnvironment = false; + bool requireEnvironmentCtor = false; + bool requireEnvironmentCreate = false; var parmTypesWithEnv = Utils.Concat(new Type[1] { typeof(IHostEnvironment) }, parmTypes); + if (Utils.Size(parmTypes) == 0 && (getter = FindInstanceGetter(instType, loaderType)) != null) return true; - if (instType.IsAssignableFrom(loaderType) && (ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypes ?? Type.EmptyTypes, null)) != null) - return true; - if (instType.IsAssignableFrom(loaderType) && (ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypesWithEnv ?? Type.EmptyTypes, null)) != null) + + if (instType.IsAssignableFrom(loaderType)) { - requireEnvironment = true; - return true; + ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypes ?? Type.EmptyTypes, null); + if (ctor == null) + { + ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypesWithEnv ?? Type.EmptyTypes, null); + if (ctor != null) + requireEnvironmentCtor = true; + } + } + + create = FindCreateMethod(instType, loaderType, parmTypes ?? Type.EmptyTypes); + if (create == null) + { + create = FindCreateMethod(instType, loaderType, parmTypesWithEnv ?? Type.EmptyTypes); + if (create != null) + requireEnvironmentCreate = true; + } + + if(ctor != null && create != null) + { + if (ctor.IsPublic == create.IsPublic) + { + var myPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsPublic.txt"; + var myNonPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsNotPublic.txt"; + var myLogPath = ctor.IsPublic ? myPublicLogPath : myNonPublicLogPath; + + MyLogLabel: // to retry in case some other process is logging to that file + try + { + using(var file = new System.IO.StreamWriter(myLogPath, true)) + { + file.WriteLine($"{loaderType.ToString()}"); + + } + } + catch(System.IO.IOException) + { + goto MyLogLabel; + } + + // For this experiment simply continue with the constructor: + requireEnvironment = requireEnvironmentCtor; + create = null; + return true; + + // throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); + } + else if (ctor.IsPublic) + { + create = null; + requireEnvironment = requireEnvironmentCtor; + return true; + } + else + { + ctor = null; + requireEnvironment = requireEnvironmentCreate; + return true; + } } - if ((create = FindCreateMethod(instType, loaderType, parmTypes ?? Type.EmptyTypes)) != null) + else if (ctor != null && create == null) + { + requireEnvironment = requireEnvironmentCtor; return true; - if ((create = FindCreateMethod(instType, loaderType, parmTypesWithEnv ?? Type.EmptyTypes)) != null) + } + else if (ctor == null && create != null) { - requireEnvironment = true; + requireEnvironment = requireEnvironmentCreate; return true; } From e7f0910083bff3e94952c70b9baef612d76b29a1 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Fri, 8 Nov 2019 13:16:12 -0800 Subject: [PATCH 02/14] Also log parmTypes --- src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 55d2ccee3c..5181076f72 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -453,7 +453,8 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp { using(var file = new System.IO.StreamWriter(myLogPath, true)) { - file.WriteLine($"{loaderType.ToString()}"); + var parmsString = string.Join(", ", parmTypes); + file.WriteLine($"{loaderType.ToString()}\t\t- parmTypes: {parmsString}\t\t- reqEnvCtor: {requireEnvironmentCtor} - reqEnvCreate: {requireEnvironmentCreate}"); } } From 1e5170acf53fd596f7098b577e8c9359fde9c37c Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 10:53:17 -0800 Subject: [PATCH 03/14] Changed access modifiers of some classes --- src/Microsoft.ML.Data/Prediction/Calibrator.cs | 6 +++--- .../Transforms/FeatureContributionCalculationTransformer.cs | 2 +- .../Transforms/SlotsDroppingTransformer.cs | 2 +- .../MatrixFactorizationPredictor.cs | 4 ++-- .../FieldAwareFactorizationMachineModelParameters.cs | 6 +++--- .../Standard/LinearModelParameters.cs | 6 +++--- .../LogisticRegression/MulticlassLogisticRegression.cs | 2 +- .../MulticlassClassification/MulticlassNaiveBayesTrainer.cs | 2 +- .../MulticlassClassification/OneVersusAllTrainer.cs | 2 +- .../MulticlassClassification/PairwiseCouplingTrainer.cs | 2 +- .../Standard/Simple/SimpleTrainers.cs | 4 ++-- src/Microsoft.ML.Transforms/FourierDistributionSampler.cs | 4 ++-- .../MissingValueDroppingTransformer.cs | 2 +- src/Microsoft.ML.Vision/ImageClassificationTrainer.cs | 2 +- 14 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.ML.Data/Prediction/Calibrator.cs b/src/Microsoft.ML.Data/Prediction/Calibrator.cs index c4cc248dc7..b2f749a69a 100644 --- a/src/Microsoft.ML.Data/Prediction/Calibrator.cs +++ b/src/Microsoft.ML.Data/Prediction/Calibrator.cs @@ -1170,7 +1170,7 @@ private NaiveCalibrator(IHostEnvironment env, ModelLoadContext ctx) _host.CheckDecode(_binProbs.All(x => (0 <= x && x <= 1))); } - private static NaiveCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) + internal static NaiveCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -1621,7 +1621,7 @@ private PlattCalibrator(IHostEnvironment env, ModelLoadContext ctx) _host.CheckDecode(FloatUtils.IsFinite(Offset)); } - private static PlattCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) + internal static PlattCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -1918,7 +1918,7 @@ private IsotonicCalibrator(IHostEnvironment env, ModelLoadContext ctx) _host.CheckDecode(valuePrev <= 1); } - private static IsotonicCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IsotonicCalibrator Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Data/Transforms/FeatureContributionCalculationTransformer.cs b/src/Microsoft.ML.Data/Transforms/FeatureContributionCalculationTransformer.cs index e8c07e1b8b..b61ce5cf68 100644 --- a/src/Microsoft.ML.Data/Transforms/FeatureContributionCalculationTransformer.cs +++ b/src/Microsoft.ML.Data/Transforms/FeatureContributionCalculationTransformer.cs @@ -148,7 +148,7 @@ private protected override void SaveModel(ModelSaveContext ctx) } // Factory method for SignatureLoadModel. - private static FeatureContributionCalculatingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FeatureContributionCalculatingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); ctx.CheckAtModel(GetVersionInfo()); diff --git a/src/Microsoft.ML.Data/Transforms/SlotsDroppingTransformer.cs b/src/Microsoft.ML.Data/Transforms/SlotsDroppingTransformer.cs index 33baa9751a..1b759ff9b6 100644 --- a/src/Microsoft.ML.Data/Transforms/SlotsDroppingTransformer.cs +++ b/src/Microsoft.ML.Data/Transforms/SlotsDroppingTransformer.cs @@ -302,7 +302,7 @@ private SlotsDroppingTransformer(IHostEnvironment env, ModelLoadContext ctx) } // Factory method for SignatureLoadModel. - private static SlotsDroppingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static SlotsDroppingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); ctx.CheckAtModel(GetVersionInfo()); diff --git a/src/Microsoft.ML.Recommender/MatrixFactorizationPredictor.cs b/src/Microsoft.ML.Recommender/MatrixFactorizationPredictor.cs index 039667d5d5..3e7c2dfdad 100644 --- a/src/Microsoft.ML.Recommender/MatrixFactorizationPredictor.cs +++ b/src/Microsoft.ML.Recommender/MatrixFactorizationPredictor.cs @@ -152,7 +152,7 @@ private MatrixFactorizationModelParameters(IHostEnvironment env, ModelLoadContex /// /// Load model from the given context /// - private static MatrixFactorizationModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static MatrixFactorizationModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -556,7 +556,7 @@ private static VersionInfo GetVersionInfo() loaderSignature: LoaderSignature, loaderAssemblyName: typeof(MatrixFactorizationPredictionTransformer).Assembly.FullName); } - private static MatrixFactorizationPredictionTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static MatrixFactorizationPredictionTransformer Create(IHostEnvironment env, ModelLoadContext ctx) => new MatrixFactorizationPredictionTransformer(env, ctx); } diff --git a/src/Microsoft.ML.StandardTrainers/FactorizationMachine/FieldAwareFactorizationMachineModelParameters.cs b/src/Microsoft.ML.StandardTrainers/FactorizationMachine/FieldAwareFactorizationMachineModelParameters.cs index b63ac61ef2..45791efe88 100644 --- a/src/Microsoft.ML.StandardTrainers/FactorizationMachine/FieldAwareFactorizationMachineModelParameters.cs +++ b/src/Microsoft.ML.StandardTrainers/FactorizationMachine/FieldAwareFactorizationMachineModelParameters.cs @@ -175,7 +175,7 @@ private FieldAwareFactorizationMachineModelParameters(IHostEnvironment env, Mode } } - private static FieldAwareFactorizationMachineModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FieldAwareFactorizationMachineModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -329,7 +329,7 @@ internal FieldAwareFactorizationMachinePredictionTransformer(IHostEnvironment ho Scorer = new BinaryClassifierScorer(Host, args, new EmptyDataView(Host, trainSchema), BindableMapper.Bind(Host, schema), schema); } - internal FieldAwareFactorizationMachinePredictionTransformer(IHostEnvironment host, ModelLoadContext ctx) + private FieldAwareFactorizationMachinePredictionTransformer(IHostEnvironment host, ModelLoadContext ctx) : base(Contracts.CheckRef(host, nameof(host)).Register(nameof(FieldAwareFactorizationMachinePredictionTransformer)), ctx) { // *** Binary format *** @@ -439,7 +439,7 @@ private static VersionInfo GetVersionInfo() loaderAssemblyName: typeof(FieldAwareFactorizationMachinePredictionTransformer).Assembly.FullName); } - private static FieldAwareFactorizationMachinePredictionTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FieldAwareFactorizationMachinePredictionTransformer Create(IHostEnvironment env, ModelLoadContext ctx) => new FieldAwareFactorizationMachinePredictionTransformer(env, ctx); } } diff --git a/src/Microsoft.ML.StandardTrainers/Standard/LinearModelParameters.cs b/src/Microsoft.ML.StandardTrainers/Standard/LinearModelParameters.cs index 27d9ad6f25..3af28ad5fd 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/LinearModelParameters.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/LinearModelParameters.cs @@ -470,7 +470,7 @@ private LinearBinaryModelParameters(IHostEnvironment env, ModelLoadContext ctx) } } - private static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -633,7 +633,7 @@ private LinearRegressionModelParameters(IHostEnvironment env, ModelLoadContext c { } - private static LinearRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static LinearRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -719,7 +719,7 @@ private PoissonRegressionModelParameters(IHostEnvironment env, ModelLoadContext { } - private static PoissonRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static PoissonRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.StandardTrainers/Standard/LogisticRegression/MulticlassLogisticRegression.cs b/src/Microsoft.ML.StandardTrainers/Standard/LogisticRegression/MulticlassLogisticRegression.cs index 3602ba9adc..9ea5cebd68 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/LogisticRegression/MulticlassLogisticRegression.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/LogisticRegression/MulticlassLogisticRegression.cs @@ -1148,7 +1148,7 @@ private protected override void Calibrate(Span dst) { } - private static LinearMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static LinearMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs index 28e673ed64..ac5408772b 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/MulticlassNaiveBayesTrainer.cs @@ -319,7 +319,7 @@ private NaiveBayesMulticlassModelParameters(IHostEnvironment env, ModelLoadConte _outputType = new VectorDataViewType(NumberDataViewType.Single, _labelCount); } - private static NaiveBayesMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static NaiveBayesMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/OneVersusAllTrainer.cs b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/OneVersusAllTrainer.cs index 2ae05af908..3a7bcea361 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/OneVersusAllTrainer.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/OneVersusAllTrainer.cs @@ -386,7 +386,7 @@ private OneVersusAllModelParameters(IHostEnvironment env, ModelLoadContext ctx) DistType = new VectorDataViewType(NumberDataViewType.Single, _impl.Predictors.Length); } - private static OneVersusAllModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static OneVersusAllModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/PairwiseCouplingTrainer.cs b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/PairwiseCouplingTrainer.cs index 09bdcc8fd1..0d76719833 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/PairwiseCouplingTrainer.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/MulticlassClassification/PairwiseCouplingTrainer.cs @@ -341,7 +341,7 @@ private bool IsValid(IValueMapperDist mapper, ref VectorDataViewType inputType) return true; } - private static PairwiseCouplingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static PairwiseCouplingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.StandardTrainers/Standard/Simple/SimpleTrainers.cs b/src/Microsoft.ML.StandardTrainers/Standard/Simple/SimpleTrainers.cs index df9ceb78d1..9f90651436 100644 --- a/src/Microsoft.ML.StandardTrainers/Standard/Simple/SimpleTrainers.cs +++ b/src/Microsoft.ML.StandardTrainers/Standard/Simple/SimpleTrainers.cs @@ -97,7 +97,7 @@ private RandomModelParameters(IHostEnvironment env, ModelLoadContext ctx) _inputType = new VectorDataViewType(NumberDataViewType.Single); } - private static RandomModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static RandomModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -377,7 +377,7 @@ private PriorModelParameters(IHostEnvironment env, ModelLoadContext ctx) _inputType = new VectorDataViewType(NumberDataViewType.Single); } - private static PriorModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static PriorModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Transforms/FourierDistributionSampler.cs b/src/Microsoft.ML.Transforms/FourierDistributionSampler.cs index 9a2f54ec9a..ec6bcf830c 100644 --- a/src/Microsoft.ML.Transforms/FourierDistributionSampler.cs +++ b/src/Microsoft.ML.Transforms/FourierDistributionSampler.cs @@ -144,7 +144,7 @@ public RandomNumberGenerator(float gamma, float averageDistance) _gamma = gamma / averageDistance; } - private static RandomNumberGenerator Create(IHostEnvironment env, ModelLoadContext ctx) + internal static RandomNumberGenerator Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -266,7 +266,7 @@ public RandomNumberGenerator(float a, float averageDistance) _a = a / averageDistance; } - private static RandomNumberGenerator Create(IHostEnvironment env, ModelLoadContext ctx) + internal static RandomNumberGenerator Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Transforms/MissingValueDroppingTransformer.cs b/src/Microsoft.ML.Transforms/MissingValueDroppingTransformer.cs index c6fec9dbee..cf5c43ad9e 100644 --- a/src/Microsoft.ML.Transforms/MissingValueDroppingTransformer.cs +++ b/src/Microsoft.ML.Transforms/MissingValueDroppingTransformer.cs @@ -109,7 +109,7 @@ private protected override void CheckInputColumn(DataViewSchema inputSchema, int } // Factory method for SignatureLoadModel - private static MissingValueDroppingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static MissingValueDroppingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); ctx.CheckAtModel(GetVersionInfo()); diff --git a/src/Microsoft.ML.Vision/ImageClassificationTrainer.cs b/src/Microsoft.ML.Vision/ImageClassificationTrainer.cs index 23d7eece24..03892d1fae 100644 --- a/src/Microsoft.ML.Vision/ImageClassificationTrainer.cs +++ b/src/Microsoft.ML.Vision/ImageClassificationTrainer.cs @@ -1448,7 +1448,7 @@ private ImageClassificationModelParameters(IHostEnvironment env, ModelLoadContex _outputType = new VectorDataViewType(NumberDataViewType.Single, _classCount); } - private static ImageClassificationModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static ImageClassificationModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); From b1e07619a7d45780ac8f7c01add32b4ad571a77f Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 11:26:35 -0800 Subject: [PATCH 04/14] Added filtering between nonpublic methods --- .../ComponentModel/ComponentCatalog.cs | 57 +++++++++++++------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 5181076f72..d07386a17e 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -440,35 +440,38 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp requireEnvironmentCreate = true; } + // If both 'ctor' and 'create' methods were found + // Choose the public one, or the internal one, and at last the private one + // if they have the same visibility, then throw an exception, since this + // shouldn't happen. if(ctor != null && create != null) { if (ctor.IsPublic == create.IsPublic) { - var myPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsPublic.txt"; - var myNonPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsNotPublic.txt"; - var myLogPath = ctor.IsPublic ? myPublicLogPath : myNonPublicLogPath; - - MyLogLabel: // to retry in case some other process is logging to that file - try + if(ctor.IsPublic || ctor.IsPrivate == create.IsPrivate) { - using(var file = new System.IO.StreamWriter(myLogPath, true)) - { - var parmsString = string.Join(", ", parmTypes); - file.WriteLine($"{loaderType.ToString()}\t\t- parmTypes: {parmsString}\t\t- reqEnvCtor: {requireEnvironmentCtor} - reqEnvCreate: {requireEnvironmentCreate}"); - - } + var myPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsPublic.txt"; + var myNonPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsNotPublic.txt"; + var myLogPath = ctor.IsPublic ? myPublicLogPath : myNonPublicLogPath; + MyLog(myLogPath, loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); + // For this experiment simply continue with the constructor: + requireEnvironment = requireEnvironmentCtor; + create = null; + return true; + // throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); } - catch(System.IO.IOException) + + if(!ctor.IsPrivate) { - goto MyLogLabel; + create = null; + requireEnvironment = requireEnvironmentCtor; + return true; } - // For this experiment simply continue with the constructor: - requireEnvironment = requireEnvironmentCtor; - create = null; + ctor = null; + requireEnvironment = requireEnvironmentCreate; return true; - // throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); } else if (ctor.IsPublic) { @@ -497,6 +500,24 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp return false; } + private static void MyLog(string path, Type loaderType, Type[] parmTypes, bool reqCtor, bool reqCreate) + { + MyLogLabel: // to retry in case some other process is logging to that file + try + { + using (var file = new System.IO.StreamWriter(path, true)) + { + var parmsString = string.Join(", ", parmTypes); + file.WriteLine($"{loaderType.ToString()}\t\t- parmTypes: {parmsString}\t\t- reqEnvCtor: {reqCtor} - reqEnvCreate: {reqCreate}"); + + } + } + catch (System.IO.IOException) + { + goto MyLogLabel; + } + } + private void AddClass(LoadableClassInfo info, string[] loadNames, bool throwOnError) { _classes.Add(info); From a1a430d9fca99b2febc2a2cf7fd9b798f3e25fe5 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 12:41:07 -0800 Subject: [PATCH 05/14] Changed visibility of several classes --- .../Trainer/EnsembleDistributionModelParameters.cs | 2 +- src/Microsoft.ML.Ensemble/Trainer/EnsembleModelParameters.cs | 2 +- .../Trainer/Multiclass/EnsembleMulticlassModelParameters.cs | 2 +- src/Microsoft.ML.FastTree/FastTreeClassification.cs | 2 +- src/Microsoft.ML.FastTree/FastTreeRanking.cs | 2 +- src/Microsoft.ML.FastTree/FastTreeRegression.cs | 2 +- src/Microsoft.ML.FastTree/FastTreeTweedie.cs | 2 +- src/Microsoft.ML.FastTree/GamClassification.cs | 2 +- src/Microsoft.ML.FastTree/GamRegression.cs | 2 +- src/Microsoft.ML.FastTree/RandomForestClassification.cs | 2 +- src/Microsoft.ML.FastTree/RandomForestRegression.cs | 2 +- .../TreeEnsembleFeaturizationTransformer.cs | 2 +- src/Microsoft.ML.KMeansClustering/KMeansModelParameters.cs | 2 +- src/Microsoft.ML.LightGbm/LightGbmBinaryTrainer.cs | 2 +- src/Microsoft.ML.LightGbm/LightGbmRankingTrainer.cs | 2 +- src/Microsoft.ML.LightGbm/LightGbmRegressionTrainer.cs | 2 +- src/Microsoft.ML.Mkl.Components/OlsLinearRegression.cs | 2 +- src/Microsoft.ML.PCA/PcaTrainer.cs | 2 +- src/Microsoft.ML.TimeSeries/IidChangePointDetector.cs | 4 ++-- src/Microsoft.ML.TimeSeries/IidSpikeDetector.cs | 4 ++-- src/Microsoft.ML.TimeSeries/SRCNNAnomalyDetector.cs | 4 ++-- src/Microsoft.ML.TimeSeries/SSaForecasting.cs | 4 ++-- src/Microsoft.ML.TimeSeries/SsaChangePointDetector.cs | 4 ++-- src/Microsoft.ML.TimeSeries/SsaSpikeDetector.cs | 4 ++-- 24 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.ML.Ensemble/Trainer/EnsembleDistributionModelParameters.cs b/src/Microsoft.ML.Ensemble/Trainer/EnsembleDistributionModelParameters.cs index 0d60f1e5f3..64ee2b84e2 100644 --- a/src/Microsoft.ML.Ensemble/Trainer/EnsembleDistributionModelParameters.cs +++ b/src/Microsoft.ML.Ensemble/Trainer/EnsembleDistributionModelParameters.cs @@ -119,7 +119,7 @@ private bool IsValid(IValueMapperDist mapper, out VectorDataViewType inputType) } } - private static EnsembleDistributionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static EnsembleDistributionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Ensemble/Trainer/EnsembleModelParameters.cs b/src/Microsoft.ML.Ensemble/Trainer/EnsembleModelParameters.cs index 945f8164e1..4db10bf000 100644 --- a/src/Microsoft.ML.Ensemble/Trainer/EnsembleModelParameters.cs +++ b/src/Microsoft.ML.Ensemble/Trainer/EnsembleModelParameters.cs @@ -109,7 +109,7 @@ private bool IsValid(IValueMapper mapper, out VectorDataViewType inputType) } } - private static EnsembleModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static EnsembleModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Ensemble/Trainer/Multiclass/EnsembleMulticlassModelParameters.cs b/src/Microsoft.ML.Ensemble/Trainer/Multiclass/EnsembleMulticlassModelParameters.cs index 08948e9fa7..2a0987091f 100644 --- a/src/Microsoft.ML.Ensemble/Trainer/Multiclass/EnsembleMulticlassModelParameters.cs +++ b/src/Microsoft.ML.Ensemble/Trainer/Multiclass/EnsembleMulticlassModelParameters.cs @@ -91,7 +91,7 @@ private void InitializeMappers(out IValueMapper[] mappers, out VectorDataViewTyp inputType = new VectorDataViewType(NumberDataViewType.Single); } - private static EnsembleMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static EnsembleMulticlassModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/FastTreeClassification.cs b/src/Microsoft.ML.FastTree/FastTreeClassification.cs index a75d061920..1df0203879 100644 --- a/src/Microsoft.ML.FastTree/FastTreeClassification.cs +++ b/src/Microsoft.ML.FastTree/FastTreeClassification.cs @@ -84,7 +84,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/FastTreeRanking.cs b/src/Microsoft.ML.FastTree/FastTreeRanking.cs index 158271af86..f484d1987e 100644 --- a/src/Microsoft.ML.FastTree/FastTreeRanking.cs +++ b/src/Microsoft.ML.FastTree/FastTreeRanking.cs @@ -1170,7 +1170,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static FastTreeRankingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FastTreeRankingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { return new FastTreeRankingModelParameters(env, ctx); } diff --git a/src/Microsoft.ML.FastTree/FastTreeRegression.cs b/src/Microsoft.ML.FastTree/FastTreeRegression.cs index 400a82b747..fce15f7ccf 100644 --- a/src/Microsoft.ML.FastTree/FastTreeRegression.cs +++ b/src/Microsoft.ML.FastTree/FastTreeRegression.cs @@ -513,7 +513,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static FastTreeRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FastTreeRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/FastTreeTweedie.cs b/src/Microsoft.ML.FastTree/FastTreeTweedie.cs index bda0271782..a31556d0a6 100644 --- a/src/Microsoft.ML.FastTree/FastTreeTweedie.cs +++ b/src/Microsoft.ML.FastTree/FastTreeTweedie.cs @@ -522,7 +522,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static FastTreeTweedieModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FastTreeTweedieModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/GamClassification.cs b/src/Microsoft.ML.FastTree/GamClassification.cs index f03821ebe4..a849758728 100644 --- a/src/Microsoft.ML.FastTree/GamClassification.cs +++ b/src/Microsoft.ML.FastTree/GamClassification.cs @@ -230,7 +230,7 @@ private static VersionInfo GetVersionInfo() loaderAssemblyName: typeof(GamBinaryModelParameters).Assembly.FullName); } - private static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/GamRegression.cs b/src/Microsoft.ML.FastTree/GamRegression.cs index de4e9cf63c..b6c69b7343 100644 --- a/src/Microsoft.ML.FastTree/GamRegression.cs +++ b/src/Microsoft.ML.FastTree/GamRegression.cs @@ -180,7 +180,7 @@ private static VersionInfo GetVersionInfo() loaderAssemblyName: typeof(GamRegressionModelParameters).Assembly.FullName); } - private static GamRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static GamRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/RandomForestClassification.cs b/src/Microsoft.ML.FastTree/RandomForestClassification.cs index efb8939d9d..da6472d9fa 100644 --- a/src/Microsoft.ML.FastTree/RandomForestClassification.cs +++ b/src/Microsoft.ML.FastTree/RandomForestClassification.cs @@ -99,7 +99,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/RandomForestRegression.cs b/src/Microsoft.ML.FastTree/RandomForestRegression.cs index bcfd4e0b9e..e55e236754 100644 --- a/src/Microsoft.ML.FastTree/RandomForestRegression.cs +++ b/src/Microsoft.ML.FastTree/RandomForestRegression.cs @@ -199,7 +199,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.Writer.Write(_quantileSampleCount); } - private static FastForestRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static FastForestRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.FastTree/TreeEnsembleFeaturizationTransformer.cs b/src/Microsoft.ML.FastTree/TreeEnsembleFeaturizationTransformer.cs index d30337b07e..7d516c35e8 100644 --- a/src/Microsoft.ML.FastTree/TreeEnsembleFeaturizationTransformer.cs +++ b/src/Microsoft.ML.FastTree/TreeEnsembleFeaturizationTransformer.cs @@ -178,7 +178,7 @@ private static VersionInfo GetVersionInfo() loaderAssemblyName: typeof(TreeEnsembleFeaturizationTransformer).Assembly.FullName); } - private static TreeEnsembleFeaturizationTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static TreeEnsembleFeaturizationTransformer Create(IHostEnvironment env, ModelLoadContext ctx) => new TreeEnsembleFeaturizationTransformer(env, ctx); } } \ No newline at end of file diff --git a/src/Microsoft.ML.KMeansClustering/KMeansModelParameters.cs b/src/Microsoft.ML.KMeansClustering/KMeansModelParameters.cs index db8876ab4a..dfeedde310 100644 --- a/src/Microsoft.ML.KMeansClustering/KMeansModelParameters.cs +++ b/src/Microsoft.ML.KMeansClustering/KMeansModelParameters.cs @@ -253,7 +253,7 @@ private protected override void SaveCore(ModelSaveContext ctx) /// /// This method is called by reflection to instantiate a predictor. /// - private static KMeansModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static KMeansModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.LightGbm/LightGbmBinaryTrainer.cs b/src/Microsoft.ML.LightGbm/LightGbmBinaryTrainer.cs index 22de1c03f7..a469c597f6 100644 --- a/src/Microsoft.ML.LightGbm/LightGbmBinaryTrainer.cs +++ b/src/Microsoft.ML.LightGbm/LightGbmBinaryTrainer.cs @@ -70,7 +70,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IPredictorProducing Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.LightGbm/LightGbmRankingTrainer.cs b/src/Microsoft.ML.LightGbm/LightGbmRankingTrainer.cs index 8ed23778e9..a0c0438da1 100644 --- a/src/Microsoft.ML.LightGbm/LightGbmRankingTrainer.cs +++ b/src/Microsoft.ML.LightGbm/LightGbmRankingTrainer.cs @@ -67,7 +67,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static LightGbmRankingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static LightGbmRankingModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { return new LightGbmRankingModelParameters(env, ctx); } diff --git a/src/Microsoft.ML.LightGbm/LightGbmRegressionTrainer.cs b/src/Microsoft.ML.LightGbm/LightGbmRegressionTrainer.cs index 90ad8cfa39..9e844ca8d9 100644 --- a/src/Microsoft.ML.LightGbm/LightGbmRegressionTrainer.cs +++ b/src/Microsoft.ML.LightGbm/LightGbmRegressionTrainer.cs @@ -66,7 +66,7 @@ private protected override void SaveCore(ModelSaveContext ctx) ctx.SetVersionInfo(GetVersionInfo()); } - private static LightGbmRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static LightGbmRegressionModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.Mkl.Components/OlsLinearRegression.cs b/src/Microsoft.ML.Mkl.Components/OlsLinearRegression.cs index 233435d17c..8edb230f54 100644 --- a/src/Microsoft.ML.Mkl.Components/OlsLinearRegression.cs +++ b/src/Microsoft.ML.Mkl.Components/OlsLinearRegression.cs @@ -741,7 +741,7 @@ private static void ProbCheckDecode(Double p) Contracts.CheckDecode(0 <= p && p <= 1); } - private static OlsModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static OlsModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.PCA/PcaTrainer.cs b/src/Microsoft.ML.PCA/PcaTrainer.cs index 8faf05e3f8..6f24ba8290 100644 --- a/src/Microsoft.ML.PCA/PcaTrainer.cs +++ b/src/Microsoft.ML.PCA/PcaTrainer.cs @@ -540,7 +540,7 @@ private protected override void SaveCore(ModelSaveContext ctx) writer.WriteSinglesNoCount(_eigenVectors[i].GetValues().Slice(0, _dimension)); } - private static PcaModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) + internal static PcaModelParameters Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); diff --git a/src/Microsoft.ML.TimeSeries/IidChangePointDetector.cs b/src/Microsoft.ML.TimeSeries/IidChangePointDetector.cs index fd55691959..3d018944af 100644 --- a/src/Microsoft.ML.TimeSeries/IidChangePointDetector.cs +++ b/src/Microsoft.ML.TimeSeries/IidChangePointDetector.cs @@ -146,7 +146,7 @@ private static IDataTransform Create(IHostEnvironment env, ModelLoadContext ctx, } // Factory method for SignatureLoadModel. - private static IidChangePointDetector Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IidChangePointDetector Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -155,7 +155,7 @@ private static IidChangePointDetector Create(IHostEnvironment env, ModelLoadCont return new IidChangePointDetector(env, ctx); } - internal IidChangePointDetector(IHostEnvironment env, ModelLoadContext ctx) + private IidChangePointDetector(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { // *** Binary format *** diff --git a/src/Microsoft.ML.TimeSeries/IidSpikeDetector.cs b/src/Microsoft.ML.TimeSeries/IidSpikeDetector.cs index 508cf0ec90..6431763804 100644 --- a/src/Microsoft.ML.TimeSeries/IidSpikeDetector.cs +++ b/src/Microsoft.ML.TimeSeries/IidSpikeDetector.cs @@ -128,7 +128,7 @@ private static IDataTransform Create(IHostEnvironment env, ModelLoadContext ctx, } // Factory method for SignatureLoadModel. - private static IidSpikeDetector Create(IHostEnvironment env, ModelLoadContext ctx) + internal static IidSpikeDetector Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -137,7 +137,7 @@ private static IidSpikeDetector Create(IHostEnvironment env, ModelLoadContext ct return new IidSpikeDetector(env, ctx); } - internal IidSpikeDetector(IHostEnvironment env, ModelLoadContext ctx) + private IidSpikeDetector(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { // *** Binary format *** diff --git a/src/Microsoft.ML.TimeSeries/SRCNNAnomalyDetector.cs b/src/Microsoft.ML.TimeSeries/SRCNNAnomalyDetector.cs index 31f00c10d1..a9f020c0d2 100644 --- a/src/Microsoft.ML.TimeSeries/SRCNNAnomalyDetector.cs +++ b/src/Microsoft.ML.TimeSeries/SRCNNAnomalyDetector.cs @@ -127,7 +127,7 @@ private static IDataTransform Create(IHostEnvironment env, ModelLoadContext ctx, return new SrCnnAnomalyDetector(env, ctx).MakeDataTransform(input); } - private static SrCnnAnomalyDetector Create(IHostEnvironment env, ModelLoadContext ctx) + internal static SrCnnAnomalyDetector Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -152,7 +152,7 @@ internal SrCnnAnomalyDetector(IHostEnvironment env, Options options) { } - internal SrCnnAnomalyDetector(IHostEnvironment env, ModelLoadContext ctx) + private SrCnnAnomalyDetector(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { } diff --git a/src/Microsoft.ML.TimeSeries/SSaForecasting.cs b/src/Microsoft.ML.TimeSeries/SSaForecasting.cs index 8dc9c981b3..e7584090eb 100644 --- a/src/Microsoft.ML.TimeSeries/SSaForecasting.cs +++ b/src/Microsoft.ML.TimeSeries/SSaForecasting.cs @@ -167,7 +167,7 @@ IStatefulTransformer IStatefulTransformer.Clone() } // Factory method for SignatureLoadModel. - private static SsaForecastingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) + internal static SsaForecastingTransformer Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -176,7 +176,7 @@ private static SsaForecastingTransformer Create(IHostEnvironment env, ModelLoadC return new SsaForecastingTransformer(env, ctx); } - internal SsaForecastingTransformer(IHostEnvironment env, ModelLoadContext ctx) + private SsaForecastingTransformer(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { // *** Binary format *** diff --git a/src/Microsoft.ML.TimeSeries/SsaChangePointDetector.cs b/src/Microsoft.ML.TimeSeries/SsaChangePointDetector.cs index 83eb83c37d..4af231abdf 100644 --- a/src/Microsoft.ML.TimeSeries/SsaChangePointDetector.cs +++ b/src/Microsoft.ML.TimeSeries/SsaChangePointDetector.cs @@ -156,7 +156,7 @@ private static IDataTransform Create(IHostEnvironment env, ModelLoadContext ctx, } // Factory method for SignatureLoadModel. - private static SsaChangePointDetector Create(IHostEnvironment env, ModelLoadContext ctx) + internal static SsaChangePointDetector Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -165,7 +165,7 @@ private static SsaChangePointDetector Create(IHostEnvironment env, ModelLoadCont return new SsaChangePointDetector(env, ctx); } - internal SsaChangePointDetector(IHostEnvironment env, ModelLoadContext ctx) + private SsaChangePointDetector(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { // *** Binary format *** diff --git a/src/Microsoft.ML.TimeSeries/SsaSpikeDetector.cs b/src/Microsoft.ML.TimeSeries/SsaSpikeDetector.cs index 192e27388f..15e9a5f6ee 100644 --- a/src/Microsoft.ML.TimeSeries/SsaSpikeDetector.cs +++ b/src/Microsoft.ML.TimeSeries/SsaSpikeDetector.cs @@ -139,7 +139,7 @@ IStatefulTransformer IStatefulTransformer.Clone() } // Factory method for SignatureLoadModel. - private static SsaSpikeDetector Create(IHostEnvironment env, ModelLoadContext ctx) + internal static SsaSpikeDetector Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(env, nameof(env)); env.CheckValue(ctx, nameof(ctx)); @@ -148,7 +148,7 @@ private static SsaSpikeDetector Create(IHostEnvironment env, ModelLoadContext ct return new SsaSpikeDetector(env, ctx); } - internal SsaSpikeDetector(IHostEnvironment env, ModelLoadContext ctx) + private SsaSpikeDetector(IHostEnvironment env, ModelLoadContext ctx) : base(env, ctx, LoaderSignature) { // *** Binary format *** From dd62e51e44bc15a9d730e7a3ab4efeb98b823bf3 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 12:54:22 -0800 Subject: [PATCH 06/14] Adding more logs --- src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index d07386a17e..f3d3e69af6 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -465,11 +465,13 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp { create = null; requireEnvironment = requireEnvironmentCtor; + MyLog(@"C:\Users\anvelazq\Desktop\mylogPrivate.txt", loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); return true; } ctor = null; requireEnvironment = requireEnvironmentCreate; + MyLog(@"C:\Users\anvelazq\Desktop\mylogNonPublicNonPrivate.txt", loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); return true; } From e07610198ee8e39ce76a82e7784e2cabb4e40db7 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 12:54:38 -0800 Subject: [PATCH 07/14] Change access modifier of public constructors --- src/Microsoft.ML.Data/Transforms/LabelIndicatorTransform.cs | 2 +- src/Microsoft.ML.Data/Transforms/SkipTakeFilter.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.ML.Data/Transforms/LabelIndicatorTransform.cs b/src/Microsoft.ML.Data/Transforms/LabelIndicatorTransform.cs index d1c68d06ba..c9d65b227a 100644 --- a/src/Microsoft.ML.Data/Transforms/LabelIndicatorTransform.cs +++ b/src/Microsoft.ML.Data/Transforms/LabelIndicatorTransform.cs @@ -130,7 +130,7 @@ public LabelIndicatorTransform(IHostEnvironment env, { } - public LabelIndicatorTransform(IHostEnvironment env, Options options, IDataView input) + internal LabelIndicatorTransform(IHostEnvironment env, Options options, IDataView input) : base(env, LoadName, Contracts.CheckRef(options, nameof(options)).Columns, input, TestIsMulticlassLabel) { diff --git a/src/Microsoft.ML.Data/Transforms/SkipTakeFilter.cs b/src/Microsoft.ML.Data/Transforms/SkipTakeFilter.cs index 8680dc7050..c7ffc9562e 100644 --- a/src/Microsoft.ML.Data/Transforms/SkipTakeFilter.cs +++ b/src/Microsoft.ML.Data/Transforms/SkipTakeFilter.cs @@ -101,7 +101,7 @@ private SkipTakeFilter(long skip, long take, IHostEnvironment env, IDataView inp /// Host Environment. /// Options for the skip operation. /// Input . - public SkipTakeFilter(IHostEnvironment env, SkipOptions options, IDataView input) + internal SkipTakeFilter(IHostEnvironment env, SkipOptions options, IDataView input) : this(options.Count, Options.DefaultTake, env, input) { } @@ -112,7 +112,7 @@ public SkipTakeFilter(IHostEnvironment env, SkipOptions options, IDataView input /// Host Environment. /// Options for the take operation. /// Input . - public SkipTakeFilter(IHostEnvironment env, TakeOptions options, IDataView input) + internal SkipTakeFilter(IHostEnvironment env, TakeOptions options, IDataView input) : this(Options.DefaultSkip, options.Count, env, input) { } From 87c1b5a8a4a0eb9755f1d007a218a71656160d41 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 13:39:10 -0800 Subject: [PATCH 08/14] Change access modifier of SchemaBindableCalibratedModelParameters class --- src/Microsoft.ML.Data/Prediction/Calibrator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Data/Prediction/Calibrator.cs b/src/Microsoft.ML.Data/Prediction/Calibrator.cs index 626955b58e..1757d241aa 100644 --- a/src/Microsoft.ML.Data/Prediction/Calibrator.cs +++ b/src/Microsoft.ML.Data/Prediction/Calibrator.cs @@ -760,7 +760,7 @@ private SchemaBindableCalibratedModelParameters(IHostEnvironment env, ModelLoadC _featureContribution = SubModel as IFeatureContributionMapper; } - private static CalibratedModelParametersBase Create(IHostEnvironment env, ModelLoadContext ctx) + internal static CalibratedModelParametersBase Create(IHostEnvironment env, ModelLoadContext ctx) { Contracts.CheckValue(ctx, nameof(ctx)); ctx.CheckAtModel(GetVersionInfo()); From 33fb93cdbac4c61f36f745f54edce87038fd8e9d Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 13:45:30 -0800 Subject: [PATCH 09/14] Updated comments and removed logging methods --- .../ComponentModel/ComponentCatalog.cs | 36 +++---------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index f3d3e69af6..99579110e8 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -421,6 +421,7 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp if (Utils.Size(parmTypes) == 0 && (getter = FindInstanceGetter(instType, loaderType)) != null) return true; + // Find both 'ctor' and 'create' methods if available if (instType.IsAssignableFrom(loaderType)) { ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypes ?? Type.EmptyTypes, null); @@ -441,37 +442,26 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp } // If both 'ctor' and 'create' methods were found - // Choose the public one, or the internal one, and at last the private one - // if they have the same visibility, then throw an exception, since this - // shouldn't happen. + // Choose the one that is public, if both are non-public choose the one that isn't private + // if they have the same visibility, then throw an exception, since this shouldn't happen. if(ctor != null && create != null) { if (ctor.IsPublic == create.IsPublic) { if(ctor.IsPublic || ctor.IsPrivate == create.IsPrivate) { - var myPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsPublic.txt"; - var myNonPublicLogPath = @"C:\Users\anvelazq\Desktop\mylogIsNotPublic.txt"; - var myLogPath = ctor.IsPublic ? myPublicLogPath : myNonPublicLogPath; - MyLog(myLogPath, loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); - // For this experiment simply continue with the constructor: - requireEnvironment = requireEnvironmentCtor; - create = null; - return true; - // throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); + throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); } if(!ctor.IsPrivate) { create = null; requireEnvironment = requireEnvironmentCtor; - MyLog(@"C:\Users\anvelazq\Desktop\mylogPrivate.txt", loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); return true; } ctor = null; requireEnvironment = requireEnvironmentCreate; - MyLog(@"C:\Users\anvelazq\Desktop\mylogNonPublicNonPrivate.txt", loaderType, parmTypes, requireEnvironmentCtor, requireEnvironmentCreate); return true; } @@ -502,24 +492,6 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp return false; } - private static void MyLog(string path, Type loaderType, Type[] parmTypes, bool reqCtor, bool reqCreate) - { - MyLogLabel: // to retry in case some other process is logging to that file - try - { - using (var file = new System.IO.StreamWriter(path, true)) - { - var parmsString = string.Join(", ", parmTypes); - file.WriteLine($"{loaderType.ToString()}\t\t- parmTypes: {parmsString}\t\t- reqEnvCtor: {reqCtor} - reqEnvCreate: {reqCreate}"); - - } - } - catch (System.IO.IOException) - { - goto MyLogLabel; - } - } - private void AddClass(LoadableClassInfo info, string[] loadNames, bool throwOnError) { _classes.Add(info); From bb408f47ead35c2352f20f01952094741c12eaf1 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 16:03:50 -0800 Subject: [PATCH 10/14] Change if statements for 'AccessModifier' extension method --- .../ComponentModel/ComponentCatalog.cs | 71 +++++++++++++------ 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 99579110e8..a347a3e73a 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -13,6 +13,49 @@ namespace Microsoft.ML.Runtime { + + internal static class Extension + { + internal static AccessModifier Accessmodifier(this MethodInfo methodInfo) + { + if (methodInfo.IsPrivate) + return AccessModifier.Private; + if (methodInfo.IsFamily) + return AccessModifier.Protected; + if (methodInfo.IsFamilyOrAssembly) + return AccessModifier.ProtectedInternal; + if (methodInfo.IsAssembly) + return AccessModifier.Internal; + if (methodInfo.IsPublic) + return AccessModifier.Public; + throw new ArgumentException("Did not find access modifier", "methodInfo"); + } + + internal static AccessModifier Accessmodifier(this ConstructorInfo constructorInfo) + { + if (constructorInfo.IsPrivate) + return AccessModifier.Private; + if (constructorInfo.IsFamily) + return AccessModifier.Protected; + if (constructorInfo.IsFamilyOrAssembly) + return AccessModifier.ProtectedInternal; + if (constructorInfo.IsAssembly) + return AccessModifier.Internal; + if (constructorInfo.IsPublic) + return AccessModifier.Public; + throw new ArgumentException("Did not find access modifier", "methodInfo"); + } + + internal enum AccessModifier + { + Private, + Protected, + ProtectedInternal, + Internal, + Public + } + } + /// /// This catalogs instantiatable components (aka, loadable classes). Components are registered via /// a descendant of , identifying the names and signature types under which the component @@ -442,30 +485,16 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp } // If both 'ctor' and 'create' methods were found - // Choose the one that is public, if both are non-public choose the one that isn't private - // if they have the same visibility, then throw an exception, since this shouldn't happen. - if(ctor != null && create != null) + // Choose the one that is 'more' public + // If they have the same visibility, then throw an exception, since this shouldn't happen. + + if (ctor != null && create != null) { - if (ctor.IsPublic == create.IsPublic) + if (ctor.Accessmodifier() == create.Accessmodifier()) { - if(ctor.IsPublic || ctor.IsPrivate == create.IsPrivate) - { - throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); - } - - if(!ctor.IsPrivate) - { - create = null; - requireEnvironment = requireEnvironmentCtor; - return true; - } - - ctor = null; - requireEnvironment = requireEnvironmentCreate; - return true; - + throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); } - else if (ctor.IsPublic) + else if (ctor.Accessmodifier() > create.Accessmodifier()) { create = null; requireEnvironment = requireEnvironmentCtor; From 26609e84fb2b3a2f18608d7b68409432f83e2767 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Mon, 18 Nov 2019 17:08:09 -0800 Subject: [PATCH 11/14] Added PrivateProtected to enum and other minor changes --- .../ComponentModel/ComponentCatalog.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index a347a3e73a..7712e569c2 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -18,6 +18,8 @@ internal static class Extension { internal static AccessModifier Accessmodifier(this MethodInfo methodInfo) { + if (methodInfo.IsFamilyAndAssembly) + return AccessModifier.PrivateProtected; if (methodInfo.IsPrivate) return AccessModifier.Private; if (methodInfo.IsFamily) @@ -33,6 +35,8 @@ internal static AccessModifier Accessmodifier(this MethodInfo methodInfo) internal static AccessModifier Accessmodifier(this ConstructorInfo constructorInfo) { + if (constructorInfo.IsFamilyAndAssembly) + return AccessModifier.PrivateProtected; if (constructorInfo.IsPrivate) return AccessModifier.Private; if (constructorInfo.IsFamily) @@ -43,11 +47,12 @@ internal static AccessModifier Accessmodifier(this ConstructorInfo constructorIn return AccessModifier.Internal; if (constructorInfo.IsPublic) return AccessModifier.Public; - throw new ArgumentException("Did not find access modifier", "methodInfo"); + throw new ArgumentException("Did not find access modifier", "constructorInfo"); } internal enum AccessModifier { + PrivateProtected, Private, Protected, ProtectedInternal, @@ -484,12 +489,12 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp requireEnvironmentCreate = true; } - // If both 'ctor' and 'create' methods were found - // Choose the one that is 'more' public - // If they have the same visibility, then throw an exception, since this shouldn't happen. - if (ctor != null && create != null) { + // If both 'ctor' and 'create' methods were found + // Choose the one that is 'more' public + // If they have the same visibility, then throw an exception, since this shouldn't happen. + if (ctor.Accessmodifier() == create.Accessmodifier()) { throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); From ad833adc31e9a9ea91eada86ac872e5e2c3ecd31 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Tue, 26 Nov 2019 15:48:03 -0800 Subject: [PATCH 12/14] Change exception message --- src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 7712e569c2..6067cbee62 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -497,7 +497,7 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp if (ctor.Accessmodifier() == create.Accessmodifier()) { - throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please open an issue for this to be fixed."); + throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please indicate which one should be used by changing either the signature or the visibility of one of them."); } else if (ctor.Accessmodifier() > create.Accessmodifier()) { From 88aef2106480c51bd5af98ffe13c5b02d73a8f01 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Tue, 26 Nov 2019 15:51:59 -0800 Subject: [PATCH 13/14] Moving assignement of create and ctor inside if condition --- .../ComponentModel/ComponentCatalog.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 6067cbee62..23a0e71121 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -472,20 +472,16 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp // Find both 'ctor' and 'create' methods if available if (instType.IsAssignableFrom(loaderType)) { - ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypes ?? Type.EmptyTypes, null); - if (ctor == null) + if ((ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypes ?? Type.EmptyTypes, null)) == null) { - ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypesWithEnv ?? Type.EmptyTypes, null); - if (ctor != null) + if ((ctor = loaderType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, parmTypesWithEnv ?? Type.EmptyTypes, null)) != null) requireEnvironmentCtor = true; } } - create = FindCreateMethod(instType, loaderType, parmTypes ?? Type.EmptyTypes); - if (create == null) + if ((create = FindCreateMethod(instType, loaderType, parmTypes ?? Type.EmptyTypes)) == null) { - create = FindCreateMethod(instType, loaderType, parmTypesWithEnv ?? Type.EmptyTypes); - if (create != null) + if ((create = FindCreateMethod(instType, loaderType, parmTypesWithEnv ?? Type.EmptyTypes)) != null) requireEnvironmentCreate = true; } From 1dd3e33c1635e7133547787d4b880042c3f97a96 Mon Sep 17 00:00:00 2001 From: Antonio Velazquez Date: Tue, 26 Nov 2019 15:53:54 -0800 Subject: [PATCH 14/14] Erased redundant 'else' statements since 'if' statements where already returning or throwing something --- .../ComponentModel/ComponentCatalog.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs index 23a0e71121..4a7e211007 100644 --- a/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs +++ b/src/Microsoft.ML.Core/ComponentModel/ComponentCatalog.cs @@ -495,25 +495,24 @@ private static bool TryGetIniters(Type instType, Type loaderType, Type[] parmTyp { throw Contracts.Except($"Can't load type {instType}, because it has both create and constructor methods with the same visibility. Please indicate which one should be used by changing either the signature or the visibility of one of them."); } - else if (ctor.Accessmodifier() > create.Accessmodifier()) + if (ctor.Accessmodifier() > create.Accessmodifier()) { create = null; requireEnvironment = requireEnvironmentCtor; return true; } - else - { - ctor = null; - requireEnvironment = requireEnvironmentCreate; - return true; - } + ctor = null; + requireEnvironment = requireEnvironmentCreate; + return true; } - else if (ctor != null && create == null) + + if (ctor != null && create == null) { requireEnvironment = requireEnvironmentCtor; return true; } - else if (ctor == null && create != null) + + if (ctor == null && create != null) { requireEnvironment = requireEnvironmentCreate; return true;