Address some code review comments, add a non-generic base class for calibrated predictor

Author: yaeldMS

src/Microsoft.ML.Data/DataLoadSave/CompositeDataLoader.cs

@@ -98,29 +98,6 @@ void ICanSaveModel.Save(ModelSaveContext ctx)
             ctx.SaveModel(Transformer, TransformerDirectory);
         }
 
-        /// <summary>
-        /// Save the contents to a stream, as a "model file".
-        /// </summary>
-        public void SaveTo(IHostEnvironment env, Stream outputStream)
-        {
-            Contracts.CheckValue(env, nameof(env));
-            env.CheckValue(outputStream, nameof(outputStream));
-
-            env.Check(outputStream.CanWrite && outputStream.CanSeek, "Need a writable and seekable stream to save");
-            using (var ch = env.Start("Saving pipeline"))
-            {
-                using (var rep = RepositoryWriter.CreateNew(outputStream, ch))
-                {
-                    ch.Trace("Saving data loader");
-                    ModelSaveContext.SaveModel(rep, Loader, LoaderDirectory);
-
-                    ch.Trace("Saving transformer chain");
-                    ModelSaveContext.SaveModel(rep, Transformer, TransformerDirectory);
-                    rep.Commit();
-                }
-            }
-        }
-
         internal const string Summary = "A loader that encapsulates a loader and a transformer chain.";
 
         internal const string LoaderSignature = "CompositeLoader";

src/Microsoft.ML.Data/Model/ModelOperationsCatalog.cs

@@ -51,11 +51,11 @@ public void Save<TSource>(IDataLoader<TSource> loader, ITransformer model, Strea
         /// <returns>The loaded model.</returns>
         public ITransformer Load(Stream stream) => TransformerChain.LoadFrom(_env, stream);
 
-        public IDataLoader<IMultiStreamSource> LoadAsCompositeDataLoader(Stream stream)
+        public CompositeDataLoader<IMultiStreamSource, ITransformer> LoadAsCompositeDataLoader(Stream stream)
         {
             using (var rep = RepositoryReader.Open(stream))
             {
-                ModelLoadContext.LoadModel<IDataLoader<IMultiStreamSource>, SignatureLoadModel>(_env, out var model, rep, "Model");
+                ModelLoadContext.LoadModel<CompositeDataLoader<IMultiStreamSource, ITransformer>, SignatureLoadModel>(_env, out var model, rep, "Model");
                 return model;
             }
         }

src/Microsoft.ML.Data/Prediction/Calibrator.cs

@@ -57,7 +57,7 @@
     "Naive Calibration Executor",
     NaiveCalibrator.LoaderSignature)]
 
-[assembly: LoadableClass(typeof(ValueMapperCalibratedModelParameters<object, ICalibrator>), null, typeof(SignatureLoadModel),
+[assembly: LoadableClass(typeof(CalibratedModelParametersBase), typeof(ValueMapperCalibratedModelParameters<IPredictorProducing<float>, ICalibrator>), null, typeof(SignatureLoadModel),
     "Calibrated Predictor Executor",
     ValueMapperCalibratedModelParameters<IPredictorProducing<float>, ICalibrator>.LoaderSignature, "BulkCaliPredExec")]
 
@@ -150,6 +150,18 @@ internal interface IWeaklyTypedCalibratedModelParameters
         ICalibrator WeaklyTypedCalibrator { get; }
     }
 
+    public abstract class CalibratedModelParametersBase
+    {
+        public object SubModel { get; }
+        public ICalibrator Calibrator { get; }
+
+        private protected CalibratedModelParametersBase(object subModel, ICalibrator calibrator)
+        {
+            SubModel = subModel;
+            Calibrator = calibrator;
+        }
+    }
+
     /// <summary>
     /// Class for allowing a post-processing step, defined by <see cref="Calibrator"/>, to <see cref="SubModel"/>'s
     /// output.
@@ -161,7 +173,7 @@ internal interface IWeaklyTypedCalibratedModelParameters
     /// output value to the probability of belonging to the positive (or negative) class. Detailed math materials
     /// can be found at <a href="https://www.csie.ntu.edu.tw/~cjlin/papers/plattprob.pdf">this paper</a>.
     /// </remarks>
-    public abstract class CalibratedModelParametersBase<TSubModel, TCalibrator> :
+    public abstract class CalibratedModelParametersBase<TSubModel, TCalibrator> : CalibratedModelParametersBase,
         IDistPredictorProducing<float, float>,
         ICanSaveInIniFormat,
         ICanSaveInTextFormat,
@@ -178,11 +190,12 @@ public abstract class CalibratedModelParametersBase<TSubModel, TCalibrator> :
         /// <summary>
         /// <see cref="SubModel"/>'s output would calibrated by <see cref="Calibrator"/>.
         /// </summary>
-        public TSubModel SubModel { get; }
+        public new TSubModel SubModel { get; }
+
         /// <summary>
         /// <see cref="Calibrator"/> is used to post-process score produced by <see cref="SubModel"/>.
         /// </summary>
-        public TCalibrator Calibrator { get; }
+        public new TCalibrator Calibrator { get; }
 
         // Type-unsafed accessors of strongly-typed members.
         IPredictorProducing<float> IWeaklyTypedCalibratedModelParameters.WeaklyTypedSubModel => (IPredictorProducing<float>)SubModel;
@@ -191,6 +204,7 @@ public abstract class CalibratedModelParametersBase<TSubModel, TCalibrator> :
         PredictionKind IPredictor.PredictionKind => ((IPredictorProducing<float>)SubModel).PredictionKind;
 
         private protected CalibratedModelParametersBase(IHostEnvironment env, string name, TSubModel predictor, TCalibrator calibrator)
+            : base(predictor, calibrator)
         {
             Contracts.CheckValue(env, nameof(env));
             env.CheckNonWhiteSpace(name, nameof(name));
@@ -417,7 +431,7 @@ private ValueMapperCalibratedModelParameters(IHostEnvironment env, ModelLoadCont
         {
         }
 
-        private static ValueMapperCalibratedModelParameters<TSubModel, TCalibrator> Create(IHostEnvironment env, ModelLoadContext ctx)
+        private static CalibratedModelParametersBase Create(IHostEnvironment env, ModelLoadContext ctx)
         {
             Contracts.CheckValue(ctx, nameof(ctx));
             // Can load either the old "bulk" model or standard "cali". The two formats are identical.

test/Microsoft.ML.Tests/Scenarios/Api/Estimators/DeserializationTests.cs

@@ -49,7 +49,7 @@ public void LoadModelAndExtractPredictor()
 
             var gam = (((loadedModel as TransformerChain<ITransformer>).LastTransformer
                 as BinaryPredictionTransformer<object>).Model
-                as CalibratedModelParametersBase<object, ICalibrator>).SubModel
+                as CalibratedModelParametersBase).SubModel
                 as BinaryClassificationGamModelParameters;
             Assert.NotNull(gam);
         }
@@ -93,7 +93,7 @@ public void SaveAndLoadModelWithLoader()
             var ageIndex = FindIndex(slotNames.GetValues(), "age");
             var transformer = (loadedModel as CompositeDataLoader<IMultiStreamSource, ITransformer>).Transformer.LastTransformer;
             var gamModel = ((transformer as BinaryPredictionTransformer<object>).Model
-                as CalibratedModelParametersBase<object, ICalibrator>).SubModel
+                as CalibratedModelParametersBase).SubModel
                 as BinaryClassificationGamModelParameters;
             var ageBinUpperBounds = gamModel.GetBinUpperBounds(ageIndex);
             var ageBinEffects = gamModel.GetBinEffects(ageIndex);