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

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

@@ -54,7 +54,6 @@ public abstract class FastTreeTrainerBase<TArgs, TTransformer, TModel> :
     {
         protected readonly TArgs Args;
         protected readonly bool AllowGC;
-        [BestFriend]
         internal TreeEnsemble TrainedEnsemble;
         protected int FeatureCount;
         private protected RoleMappedData ValidData;
@@ -63,8 +62,8 @@ public abstract class FastTreeTrainerBase<TArgs, TTransformer, TModel> :
         /// <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>
@@ -175,8 +174,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);
 
@@ -793,7 +792,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)
         {
         }
 
@@ -2810,10 +2809,13 @@ public abstract class TreeEnsembleModelParameters :
         ISingleCanSavePfa,
         ISingleCanSaveOnnx
     {
-        public TreeEnsembleView TrainedTreeEnsembleView { get; }
+        /// <summary>
+        /// An ensemble of trees exposed to users. It is a wrapper on an <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.
@@ -2866,7 +2868,7 @@ internal TreeEnsembleModelParameters(IHostEnvironment env, string name, TreeEnse
             // 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;
 
@@ -2894,7 +2896,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

@@ -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

@@ -189,7 +189,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)
@@ -373,7 +373,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)
@@ -468,7 +468,7 @@ protected override SchemaShape.Column[] GetOutputColumnsCore(SchemaShape inputSc
             };
         }
 
-        public sealed class LambdaRankObjectiveFunction : ObjectiveFunctionBase, IStepSearch
+        internal sealed class LambdaRankObjectiveFunction : ObjectiveFunctionBase, IStepSearch
         {
             private readonly short[] _labels;
 
@@ -991,7 +991,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;

src/Microsoft.ML.FastTree/FastTreeRegression.cs

@@ -123,7 +123,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)

src/Microsoft.ML.FastTree/FastTreeTweedie.cs

@@ -128,7 +128,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)

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/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/LassoBasedEnsembleCompressor.cs

@@ -15,7 +15,6 @@ namespace Microsoft.ML.Trainers.FastTree.Internal
     /// https://www-stat.stanford.edu/~hastie/Papers/glmnet.pdf
     /// </summary>
     /// <remarks>Author was Yasser Ganjisaffar during his internship.</remarks>
-    [BestFriend]
     internal class LassoBasedEnsembleCompressor : IEnsembleCompressor<short>
     {
         // This module shouldn't consume more than 4GB of memory
@@ -534,7 +533,6 @@ private unsafe void LoadTargets(double[] trainScores, int bestIteration)
             }
         }
 
-        [BestFriend]
         bool IEnsembleCompressor<short>.Compress(IChannel ch, TreeEnsemble ensemble, double[] trainScores, int bestIteration, int maxTreesAfterCompression)
         {
             LoadTargets(trainScores, bestIteration);
@@ -553,7 +551,6 @@ bool IEnsembleCompressor<short>.Compress(IChannel ch, TreeEnsemble ensemble, dou
             return true;
         }
 
-        [BestFriend]
         TreeEnsemble IEnsembleCompressor<short>.GetCompressedEnsemble()
         {
             return _compressedEnsemble;

src/Microsoft.ML.FastTree/Training/OptimizationAlgorithms/AcceleratedGradientDescent.cs

@@ -5,9 +5,8 @@
 namespace Microsoft.ML.Trainers.FastTree.Internal
 {
     //Accelerated gradient descent score tracker
-    public class AcceleratedGradientDescent : GradientDescent
+    internal class AcceleratedGradientDescent : GradientDescent
     {
-        [BestFriend]
         internal AcceleratedGradientDescent(TreeEnsemble ensemble, Dataset trainData, double[] initTrainScores, IGradientAdjuster gradientWrapper)
             : base(ensemble, trainData, initTrainScores, gradientWrapper)
         {
@@ -18,7 +17,7 @@ protected override ScoreTracker ConstructScoreTracker(string name, Dataset set,
             return new AgdScoreTracker(name, set, initScores);
         }
 
-        public override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
+        internal override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
         {
             Contracts.CheckValue(ch, nameof(ch));
             AgdScoreTracker trainingScores = TrainingScores as AgdScoreTracker;
@@ -52,7 +51,7 @@ public override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatu
             return tree;
         }
 
-        public override void UpdateScores(ScoreTracker t, RegressionTree tree)
+        internal override void UpdateScores(ScoreTracker t, RegressionTree tree)
         {
             if (t == TrainingScores)
             {

src/Microsoft.ML.FastTree/Training/OptimizationAlgorithms/ConjugateGradientDescent.cs

@@ -5,7 +5,7 @@
 namespace Microsoft.ML.Trainers.FastTree.Internal
 {
     // Conjugate gradient descent
-    public class ConjugateGradientDescent : GradientDescent
+    internal class ConjugateGradientDescent : GradientDescent
     {
         private double[] _previousGradient;
         private double[] _currentGradient;
@@ -18,7 +18,7 @@ internal ConjugateGradientDescent(TreeEnsemble ensemble, Dataset trainData, doub
             _currentDk = new double[trainData.NumDocs];
         }
 
-        protected override double[] GetGradient(IChannel ch)
+        internal override double[] GetGradient(IChannel ch)
         {
             Contracts.AssertValue(ch);
             _previousGradient = _currentGradient;

src/Microsoft.ML.FastTree/Training/OptimizationAlgorithms/GradientDescent.cs

@@ -8,7 +8,7 @@
 
 namespace Microsoft.ML.Trainers.FastTree.Internal
 {
-    public class GradientDescent : OptimizationAlgorithm
+    internal class GradientDescent : OptimizationAlgorithm
     {
         private IGradientAdjuster _gradientWrapper;
 
@@ -21,7 +21,6 @@ public class GradientDescent : OptimizationAlgorithm
         private double[] _droppedScores;
         private double[] _scores;
 
-        [BestFriend]
         internal GradientDescent(TreeEnsemble ensemble, Dataset trainData, double[] initTrainScores, IGradientAdjuster gradientWrapper)
             : base(ensemble, trainData, initTrainScores)
         {
@@ -34,7 +33,7 @@ protected override ScoreTracker ConstructScoreTracker(string name, Dataset set,
             return new ScoreTracker(name, set, initScores);
         }
 
-        protected virtual double[] GetGradient(IChannel ch)
+        internal virtual double[] GetGradient(IChannel ch)
         {
             Contracts.AssertValue(ch);
             if (DropoutRate > 0)
@@ -91,7 +90,7 @@ protected virtual double[] AdjustTargetsAndSetWeights(IChannel ch)
             }
         }
 
-        public override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
+        internal override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
         {
             Contracts.CheckValue(ch, nameof(ch));
             // Fit a regression tree to the gradient using least squares.

src/Microsoft.ML.FastTree/Training/OptimizationAlgorithms/NoOptimizationAlgorithm.cs

@@ -8,7 +8,7 @@ namespace Microsoft.ML.Trainers.FastTree.Internal
     /// This is dummy optimizer. As Random forest does not have any boosting based optimization, this is place holder to be consistent
     /// with other fast tree based applications
     /// </summary>
-    public class RandomForestOptimizer : GradientDescent
+    internal class RandomForestOptimizer : GradientDescent
     {
         private IGradientAdjuster _gradientWrapper;
         // REVIEW: When the FastTree appliation is decoupled with tree learner and boosting logic, this class should be removed.
@@ -25,7 +25,7 @@ protected override ScoreTracker ConstructScoreTracker(string name, Dataset set,
             return new ScoreTracker(name, set, initScores);
         }
 
-        public override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
+        internal override RegressionTree TrainingIteration(IChannel ch, bool[] activeFeatures)
         {
             Contracts.CheckValue(ch, nameof(ch));