Address comments and internalize RegressionTree and TreeEsnmble.

A lot of things become internal and possibly best-friend because of
RegressionTree and TreeEnsemble.

Fix build and add some doc strings

Undo some best-friend because they have no cross-assembly reference

Author: wschin

src/Microsoft.ML.FastTree/BoostingFastTree.cs

@@ -62,7 +62,7 @@ protected override void CheckArgs(IChannel ch)
             base.CheckArgs(ch);
         }
 
-        protected override TreeLearner ConstructTreeLearner(IChannel ch)
+        internal override TreeLearner ConstructTreeLearner(IChannel ch)
         {
             return new LeastSquaresRegressionTreeLearner(
                 TrainSet, Args.NumLeaves, Args.MinDocumentsInLeafs, Args.EntropyCoefficient,
@@ -73,7 +73,7 @@ protected override TreeLearner ConstructTreeLearner(IChannel ch)
                 Args.MinDocsPercentageForCategoricalSplit, Args.Bundling, Args.MinDocsForCategoricalSplit, Args.Bias);
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             Contracts.CheckValue(ch, nameof(ch));
             OptimizationAlgorithm optimizationAlgorithm;

src/Microsoft.ML.FastTree/Dataset/FeatureFlock.cs

@@ -51,7 +51,7 @@ public PerBinStats(Double sumTargets, Double sumWeights, int count)
     /// per flock. Note that feature indices, whenever present, refer to the feature within the
     /// particular flock the same as they do with <see cref="FeatureFlockBase"/>.
     /// </summary>
-    public abstract class SufficientStatsBase
+    internal abstract class SufficientStatsBase
     {
         // REVIEW: Holdover from histogram. I really don't like this. Figure out if
         // there's a better way.
@@ -929,7 +929,7 @@ public void FillSplitCandidatesCategoricalNeighborBundling(LeastSquaresRegressio
     /// </summary>
     /// <typeparam name="TSuffStats">The type of sufficient stats that we will be able to do
     /// "peer" operations against, like subtract. This will always be the derived class itself.</typeparam>
-    public abstract class SufficientStatsBase<TSuffStats> : SufficientStatsBase
+    internal abstract class SufficientStatsBase<TSuffStats> : SufficientStatsBase
         where TSuffStats : SufficientStatsBase<TSuffStats>
     {
         protected SufficientStatsBase(int features)
@@ -1005,7 +1005,7 @@ protected FeatureFlockBase(int count, bool categorical = false)
         /// <param name="hasWeights">Whether structures related to tracking
         /// example weights should be allocated</param>
         /// <returns>A sufficient statistics object</returns>
-        public abstract SufficientStatsBase CreateSufficientStats(bool hasWeights);
+        internal abstract SufficientStatsBase CreateSufficientStats(bool hasWeights);
 
         /// <summary>
         /// Returns a forward indexer for a single feature. This has a default implementation that
@@ -1207,7 +1207,7 @@ public override long SizeInBytes()
                    + sizeof(int) * HotFeatureStarts.Length;
         }
 
-        public override SufficientStatsBase CreateSufficientStats(bool hasWeights)
+        internal override SufficientStatsBase CreateSufficientStats(bool hasWeights)
         {
             return new SufficientStats(this, hasWeights);
         }

src/Microsoft.ML.FastTree/Dataset/SingletonFeatureFlock.cs

@@ -34,7 +34,7 @@ public override long SizeInBytes()
             return _bins.SizeInBytes() + sizeof(double) * _binUpperBounds.Length;
         }
 
-        public override SufficientStatsBase CreateSufficientStats(bool hasWeights)
+        internal override SufficientStatsBase CreateSufficientStats(bool hasWeights)
         {
             return new SufficientStats(this, hasWeights);
         }

src/Microsoft.ML.FastTree/FastTree.cs

@@ -55,16 +55,16 @@ public abstract class FastTreeTrainerBase<TArgs, TTransformer, TModel> :
     {
         protected readonly TArgs Args;
         protected readonly bool AllowGC;
-        protected TreeEnsemble TrainedEnsemble;
+        internal TreeEnsemble TrainedEnsemble;
         protected int FeatureCount;
         private protected RoleMappedData ValidData;
         /// <summary>
         /// If not null, it's a test data set passed in from training context. It will be converted to one element in
         /// <see cref="Tests"/> by calling <see cref="ExamplesToFastTreeBins.GetCompatibleDataset"/> in <see cref="InitializeTests"/>.
         /// </summary>
         private protected RoleMappedData TestData;
-        protected IParallelTraining ParallelTraining;
-        protected OptimizationAlgorithm OptimizationAlgorithm;
+        internal IParallelTraining ParallelTraining;
+        internal OptimizationAlgorithm OptimizationAlgorithm;
         protected Dataset TrainSet;
         protected Dataset ValidSet;
         /// <summary>
@@ -88,7 +88,8 @@ public abstract class FastTreeTrainerBase<TArgs, TTransformer, TModel> :
         protected double[] InitValidScores;
         protected double[][] InitTestScores;
         //protected int Iteration;
-        protected TreeEnsemble Ensemble;
+        [BestFriend]
+        internal TreeEnsemble Ensemble;
 
         protected bool HasValidSet => ValidSet != null;
 
@@ -174,8 +175,8 @@ private protected FastTreeTrainerBase(IHostEnvironment env, TArgs args, SchemaSh
 
         protected abstract Test ConstructTestForTrainingData();
 
-        protected abstract OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch);
-        protected abstract TreeLearner ConstructTreeLearner(IChannel ch);
+        internal abstract OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch);
+        internal abstract TreeLearner ConstructTreeLearner(IChannel ch);
 
         protected abstract ObjectiveFunctionBase ConstructObjFunc(IChannel ch);
 
@@ -792,7 +793,7 @@ private float GetMachineAvailableBytes()
 
         // This method is called at the end of each training iteration, with the tree that was learnt on that iteration.
         // Note that this tree can be null if no tree was learnt this iteration.
-        protected virtual void CustomizedTrainingIteration(RegressionTree tree)
+        internal virtual void CustomizedTrainingIteration(RegressionTree tree)
         {
         }
 
@@ -2809,10 +2810,14 @@ public abstract class TreeEnsembleModelParameters :
         ISingleCanSavePfa,
         ISingleCanSaveOnnx
     {
-        public TreeEnsembleView TrainedTreeEnsembleView { get; }
-        //The below two properties are necessary for tree Visualizer
+        /// <summary>
+        /// An ensemble of trees exposed to users. It is a wrapper on the <see langword="internal"/> <see cref="TreeEnsemble"/> in <see cref="TreeRegressorCollection"/>.
+        /// </summary>
+        public TreeRegressorCollection TrainedTreeCollection { get; }
+
+        // The below two properties are necessary for tree Visualizer
         [BestFriend]
-        internal TreeEnsemble TrainedEnsemble => TrainedTreeEnsembleView.TreeEnsemble;
+        internal TreeEnsemble TrainedEnsemble => TrainedTreeCollection.TreeEnsemble;
         int ITreeEnsemble.NumTrees => TrainedEnsemble.NumTrees;
 
         // Inner args is used only for documentation purposes when saving comments to INI files.
@@ -2854,7 +2859,9 @@ public abstract class TreeEnsembleModelParameters :
         /// </summary>
         public FeatureContributionCalculator FeatureContributionCalculator => new FeatureContributionCalculator(this);
 
-        public TreeEnsembleModelParameters(IHostEnvironment env, string name, TreeEnsemble trainedEnsemble, int numFeatures, string innerArgs)
+        /// The following function is used in both FastTree and LightGBM so <see cref="BestFriendAttribute"/> is required.
+        [BestFriend]
+        internal TreeEnsembleModelParameters(IHostEnvironment env, string name, TreeEnsemble trainedEnsemble, int numFeatures, string innerArgs)
             : base(env, name)
         {
             Host.CheckValue(trainedEnsemble, nameof(trainedEnsemble));
@@ -2864,7 +2871,7 @@ public TreeEnsembleModelParameters(IHostEnvironment env, string name, TreeEnsemb
             // REVIEW: When we make the predictor wrapper, we may want to further "optimize"
             // the trained ensemble to, for instance, resize arrays so that they are of the length
             // the actual number of leaves/nodes, or remove unnecessary arrays, and so forth.
-            TrainedTreeEnsembleView = new TreeEnsembleView(trainedEnsemble);
+            TrainedTreeCollection = new TreeRegressorCollection(trainedEnsemble);
             InnerArgs = innerArgs;
             NumFeatures = numFeatures;
 
@@ -2892,7 +2899,7 @@ protected TreeEnsembleModelParameters(IHostEnvironment env, string name, ModelLo
             if (ctx.Header.ModelVerWritten >= VerCategoricalSplitSerialized)
                 categoricalSplits = true;
 
-            TrainedTreeEnsembleView = new TreeEnsembleView(new TreeEnsemble(ctx, usingDefaultValues, categoricalSplits));
+            TrainedTreeCollection = new TreeRegressorCollection(new TreeEnsemble(ctx, usingDefaultValues, categoricalSplits));
             MaxSplitFeatIdx = TrainedEnsemble.GetMaxFeatureIndex();
 
             InnerArgs = ctx.LoadStringOrNull();

src/Microsoft.ML.FastTree/FastTreeArguments.cs

@@ -156,7 +156,7 @@ public abstract class TreeArgs : LearnerInputBaseWithGroupId
         /// Allows to choose Parallel FastTree Learning Algorithm.
         /// </summary>
         [Argument(ArgumentType.Multiple, HelpText = "Allows to choose Parallel FastTree Learning Algorithm", ShortName = "parag")]
-        public ISupportParallelTraining ParallelTrainer = new SingleTrainerFactory();
+        internal ISupportParallelTraining ParallelTrainer = new SingleTrainerFactory();
 
         /// <summary>
         /// The number of threads to use.

src/Microsoft.ML.FastTree/FastTreeClassification.cs

@@ -70,7 +70,7 @@ private static VersionInfo GetVersionInfo()
 
         protected override uint VerCategoricalSplitSerialized => 0x00010005;
 
-        public FastTreeBinaryModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
+        internal FastTreeBinaryModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         {
         }
@@ -204,7 +204,7 @@ protected override ObjectiveFunctionBase ConstructObjFunc(IChannel ch)
                 ParallelTraining);
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             OptimizationAlgorithm optimizationAlgorithm = base.ConstructOptimizationAlgorithm(ch);
             if (Args.UseLineSearch)

src/Microsoft.ML.FastTree/FastTreeRanking.cs

@@ -190,7 +190,7 @@ protected override ObjectiveFunctionBase ConstructObjFunc(IChannel ch)
             return new LambdaRankObjectiveFunction(TrainSet, TrainSet.Ratings, Args, ParallelTraining);
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             OptimizationAlgorithm optimizationAlgorithm = base.ConstructOptimizationAlgorithm(ch);
             if (Args.UseLineSearch)
@@ -374,7 +374,7 @@ protected override void Train(IChannel ch)
             PrintTestGraph(ch);
         }
 
-        protected override void CustomizedTrainingIteration(RegressionTree tree)
+        internal override void CustomizedTrainingIteration(RegressionTree tree)
         {
             Contracts.AssertValueOrNull(tree);
             if (tree != null && Args.CompressEnsemble)
@@ -469,7 +469,7 @@ protected override SchemaShape.Column[] GetOutputColumnsCore(SchemaShape inputSc
             };
         }
 
-        public sealed class LambdaRankObjectiveFunction : ObjectiveFunctionBase, IStepSearch
+        internal sealed class LambdaRankObjectiveFunction : ObjectiveFunctionBase, IStepSearch
         {
             private readonly short[] _labels;
 
@@ -992,7 +992,7 @@ protected override void GetGradientInOneQuery(int query, int threadIndex)
                 }
             }
 
-            public void AdjustTreeOutputs(IChannel ch, RegressionTree tree, DocumentPartitioning partitioning,
+            void IStepSearch.AdjustTreeOutputs(IChannel ch, RegressionTree tree, DocumentPartitioning partitioning,
                                             ScoreTracker trainingScores)
             {
                 const double epsilon = 1.4e-45;
@@ -1131,7 +1131,7 @@ private static VersionInfo GetVersionInfo()
 
         protected override uint VerCategoricalSplitSerialized => 0x00010005;
 
-        public FastTreeRankingModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
+        internal FastTreeRankingModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         {
         }

src/Microsoft.ML.FastTree/FastTreeRegression.cs

@@ -124,7 +124,7 @@ protected override ObjectiveFunctionBase ConstructObjFunc(IChannel ch)
             return new ObjectiveImpl(TrainSet, Args);
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             OptimizationAlgorithm optimizationAlgorithm = base.ConstructOptimizationAlgorithm(ch);
             if (Args.UseLineSearch)
@@ -467,7 +467,7 @@ private static VersionInfo GetVersionInfo()
 
         protected override uint VerCategoricalSplitSerialized => 0x00010005;
 
-        public FastTreeRegressionModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
+        internal FastTreeRegressionModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         {
         }

src/Microsoft.ML.FastTree/FastTreeTweedie.cs

@@ -129,7 +129,7 @@ protected override ObjectiveFunctionBase ConstructObjFunc(IChannel ch)
             return new ObjectiveImpl(TrainSet, Args);
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             OptimizationAlgorithm optimizationAlgorithm = base.ConstructOptimizationAlgorithm(ch);
             if (Args.UseLineSearch)
@@ -470,7 +470,7 @@ private static VersionInfo GetVersionInfo()
 
         protected override uint VerCategoricalSplitSerialized => 0x00010003;
 
-        public FastTreeTweedieModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
+        internal FastTreeTweedieModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         {
         }

src/Microsoft.ML.FastTree/GamTrainer.cs

@@ -148,7 +148,7 @@ public abstract class ArgumentsBase : LearnerInputBaseWithWeight
         public override TrainerInfo Info { get; }
         private protected virtual bool NeedCalibration => false;
 
-        protected IParallelTraining ParallelTraining;
+        internal IParallelTraining ParallelTraining;
 
         private protected GamTrainerBase(IHostEnvironment env,
             string name,

src/Microsoft.ML.FastTree/Properties/AssemblyInfo.cs

@@ -15,5 +15,6 @@
 [assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Internal.FastTree" + InternalPublicKey.Value)]
 [assembly: InternalsVisibleTo(assemblyName: "RunTests" + InternalPublicKey.Value)]
 [assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Tests" + PublicKey.TestValue)]
+[assembly: InternalsVisibleTo(assemblyName: "Microsoft.ML.Predictor.Tests" + PublicKey.TestValue)]
 
 [assembly: WantsToBeBestFriends]

src/Microsoft.ML.FastTree/RandomForest.cs

@@ -42,7 +42,7 @@ protected RandomForestTrainerBase(IHostEnvironment env,
             _quantileEnabled = quantileEnabled;
         }
 
-        protected override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
+        internal override OptimizationAlgorithm ConstructOptimizationAlgorithm(IChannel ch)
         {
             Host.CheckValue(ch, nameof(ch));
             IGradientAdjuster gradientWrapper = MakeGradientWrapper(ch);
@@ -63,7 +63,7 @@ protected override void InitializeTests()
         {
         }
 
-        protected override TreeLearner ConstructTreeLearner(IChannel ch)
+        internal override TreeLearner ConstructTreeLearner(IChannel ch)
         {
             return new RandomForestLeastSquaresTreeLearner(
                        TrainSet, Args.NumLeaves, Args.MinDocumentsInLeafs, Args.EntropyCoefficient,

src/Microsoft.ML.FastTree/RandomForestClassification.cs

@@ -80,7 +80,7 @@ private static VersionInfo GetVersionInfo()
         /// </summary>
         public override PredictionKind PredictionKind => PredictionKind.BinaryClassification;
 
-        public FastForestClassificationModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
+        internal FastForestClassificationModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         { }
 

src/Microsoft.ML.FastTree/RandomForestRegression.cs

@@ -60,7 +60,7 @@ private static VersionInfo GetVersionInfo()
 
         protected override uint VerCategoricalSplitSerialized => 0x00010006;
 
-        public FastForestRegressionModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs, int samplesCount)
+        internal FastForestRegressionModelParameters(IHostEnvironment env, TreeEnsemble trainedEnsemble, int featureCount, string innerArgs, int samplesCount)
             : base(env, RegistrationName, trainedEnsemble, featureCount, innerArgs)
         {
             _quantileSampleCount = samplesCount;

src/Microsoft.ML.FastTree/Training/BaggingProvider.cs

@@ -75,7 +75,7 @@ public int GetBagCount(int numTrees, int bagSize)
         // Divides output values of leaves to bag count.
         // This brings back the final scores generated by model on a same
         // range as when we didn't use bagging
-        public void ScaleEnsembleLeaves(int numTrees, int bagSize, TreeEnsemble ensemble)
+        internal void ScaleEnsembleLeaves(int numTrees, int bagSize, TreeEnsemble ensemble)
         {
             int bagCount = GetBagCount(numTrees, bagSize);
             for (int t = 0; t < ensemble.NumTrees; t++)

src/Microsoft.ML.FastTree/Training/DocumentPartitioning.cs

@@ -50,7 +50,7 @@ public DocumentPartitioning(int[] documents, int numDocuments, int maxLeaves)
         /// Constructs partitioning object based on the documents and RegressionTree splits
         /// NOTE: It has been optimized for speed and multiprocs with 10x gain on naive LINQ implementation
         /// </summary>
-        public DocumentPartitioning(RegressionTree tree, Dataset dataset)
+        internal DocumentPartitioning(RegressionTree tree, Dataset dataset)
             : this(dataset.NumDocs, tree.NumLeaves)
         {
             using (Timer.Time(TimerEvent.DocumentPartitioningConstruction))

src/Microsoft.ML.FastTree/Training/EnsembleCompression/IEnsembleCompressor.cs

@@ -4,7 +4,7 @@
 
 namespace Microsoft.ML.Trainers.FastTree.Internal
 {
-    public interface IEnsembleCompressor<TLabel>
+    internal interface IEnsembleCompressor<TLabel>
     {
         void Initialize(int numTrees, Dataset trainSet, TLabel[] labels, int randomSeed);