review comments and further cleanup

Author: artidoro

src/Microsoft.ML.HalLearners/VectorWhitening.cs

@@ -35,9 +35,15 @@ namespace Microsoft.ML.Transforms.Projections
 {
     public enum WhiteningKind
     {
+        /// <summary>
+        /// PCA whitening.
+        /// </summary>
         [TGUI(Label = "PCA whitening")]
         Pca,
 
+        /// <summary>
+        /// ZCA whitening.
+        /// </summary>
         [TGUI(Label = "ZCA whitening")]
         Zca
     }
@@ -186,9 +192,9 @@ internal static VectorWhiteningTransformer Create(IHostEnvironment env, ModelLoa
         }
 
         // Factory method for SignatureDataTransform.
-        internal static IDataTransform Create(IHostEnvironment env, Options args, IDataView input)
+        internal static IDataTransform Create(IHostEnvironment env, Options options, IDataView input)
         {
-            var infos = args.Columns.Select(colPair => new VectorWhiteningEstimator.ColumnInfo(colPair, args)).ToArray();
+            var infos = options.Columns.Select(colPair => new VectorWhiteningEstimator.ColumnInfo(colPair, options)).ToArray();
             (var models, var invModels) = TrainVectorWhiteningTransform(env, input, infos);
             return new VectorWhiteningTransformer(env, models, invModels, infos).MakeDataTransform(input);
         }
@@ -665,7 +671,8 @@ private static float DotProduct(float[] a, int aOffset, ReadOnlySpan<float> b, R
     /// <include file='doc.xml' path='doc/members/member[@name="Whitening"]/*'/>
     public sealed class VectorWhiteningEstimator : IEstimator<VectorWhiteningTransformer>
     {
-        public static class Defaults
+        [BestFriend]
+        internal static class Defaults
         {
             public const WhiteningKind Kind = WhiteningKind.Zca;
             public const float Eps = 1e-5f;
@@ -679,11 +686,29 @@ public static class Defaults
         /// </summary>
         public sealed class ColumnInfo
         {
+            /// <summary>
+            /// Name of the column resulting from the transformation of <see cref="InputColumnName"/>.
+            /// </summary>
             public readonly string Name;
+            /// <summary>
+            /// Name of column to transform. If set to <see langword="null"/>, the value of the <see cref="Name"/> will be used as source.
+            /// </summary>
             public readonly string InputColumnName;
+            /// <summary>
+            /// Whitening kind (PCA/ZCA).
+            /// </summary>
             public readonly WhiteningKind Kind;
+            /// <summary>
+            /// Whitening constant, prevents division by zero.
+            /// </summary>
             public readonly float Epsilon;
+            /// <summary>
+            /// Maximum number of rows used to train the transform.
+            /// </summary>
             public readonly int MaxRow;
+            /// <summary>
+            /// In case of PCA whitening, indicates the number of components to retain.
+            /// </summary>
             public readonly int PcaNum;
             internal readonly bool SaveInv;
 
@@ -714,20 +739,20 @@ public ColumnInfo(string name, string inputColumnName = null, WhiteningKind kind
                 Contracts.CheckUserArg(PcaNum >= 0, nameof(PcaNum));
             }
 
-            internal ColumnInfo(VectorWhiteningTransformer.Column item, VectorWhiteningTransformer.Options args)
+            internal ColumnInfo(VectorWhiteningTransformer.Column item, VectorWhiteningTransformer.Options options)
             {
                 Name = item.Name;
                 Contracts.CheckValue(Name, nameof(Name));
                 InputColumnName = item.Source ?? item.Name;
                 Contracts.CheckValue(InputColumnName, nameof(InputColumnName));
-                Kind = item.Kind ?? args.Kind;
+                Kind = item.Kind ?? options.Kind;
                 Contracts.CheckUserArg(Kind == WhiteningKind.Pca || Kind == WhiteningKind.Zca, nameof(item.Kind));
-                Epsilon = item.Eps ?? args.Eps;
+                Epsilon = item.Eps ?? options.Eps;
                 Contracts.CheckUserArg(0 <= Epsilon && Epsilon < float.PositiveInfinity, nameof(item.Eps));
-                MaxRow = item.MaxRows ?? args.MaxRows;
+                MaxRow = item.MaxRows ?? options.MaxRows;
                 Contracts.CheckUserArg(MaxRow > 0, nameof(item.MaxRows));
-                SaveInv = item.SaveInverse ?? args.SaveInverse;
-                PcaNum = item.PcaNum ?? args.PcaNum;
+                SaveInv = item.SaveInverse ?? options.SaveInverse;
+                PcaNum = item.PcaNum ?? options.PcaNum;
                 Contracts.CheckUserArg(PcaNum >= 0, nameof(item.PcaNum));
             }
 
@@ -803,6 +828,9 @@ internal VectorWhiteningEstimator(IHostEnvironment env, string outputColumnName,
         {
         }
 
+        /// <summary>
+        /// Trains and returns a <see cref="VectorWhiteningTransformer"/>.
+        /// </summary>
         public VectorWhiteningTransformer Fit(IDataView input)
         {
             // Build transformation matrices for whitening process, then construct a trained transform.
@@ -811,7 +839,8 @@ public VectorWhiteningTransformer Fit(IDataView input)
         }
 
         /// <summary>
-        /// Returns the schema that would be produced by the transformation.
+        /// Returns the <see cref="SchemaShape"/> of the schema which will be produced by the transformer.
+        /// Used for schema propagation and verification in a pipeline.
         /// </summary>
         public SchemaShape GetOutputSchema(SchemaShape inputSchema)
         {

src/Microsoft.ML.PCA/PcaTransformer.cs

@@ -619,7 +619,8 @@ internal static CommonOutputs.TransformOutput Calculate(IHostEnvironment env, Op
     /// <include file='doc.xml' path='doc/members/member[@name="PCA"]/*'/>
     public sealed class PrincipalComponentAnalysisEstimator : IEstimator<PcaTransformer>
     {
-        public static class Defaults
+        [BestFriend]
+        internal static class Defaults
         {
             public const string WeightColumn = null;
             public const int Rank = 20;
@@ -633,12 +634,34 @@ public static class Defaults
         /// </summary>
         public sealed class ColumnInfo
         {
+            /// <summary>
+            /// Name of the column resulting from the transformation of <see cref="InputColumnName"/>.
+            /// </summary>
             public readonly string Name;
+            /// <summary>
+            /// Name of column to transform.
+            /// If set to <see langword="null"/>, the value of the <see cref="Name"/> will be used as source.
+            /// </summary>
             public readonly string InputColumnName;
+            /// <summary>
+            /// The name of the weight column.
+            /// </summary>
             public readonly string WeightColumn;
+            /// <summary>
+            /// The number of components in the PCA.
+            /// </summary>
             public readonly int Rank;
+            /// <summary>
+            /// Oversampling parameter for randomized PCA training.
+            /// </summary>
             public readonly int Oversampling;
+            /// <summary>
+            /// If enabled, data is centered to be zero mean.
+            /// </summary>
             public readonly bool Center;
+            /// <summary>
+            /// The seed for random number generation.
+            /// </summary>
             public readonly int? Seed;
 
             /// <summary>
@@ -706,7 +729,7 @@ internal PrincipalComponentAnalysisEstimator(IHostEnvironment env, params Column
         }
 
         /// <summary>
-        /// Train and return a transformer.
+        /// Trains and returns a <see cref="PcaTransformer"/>.
         /// </summary>
         public PcaTransformer Fit(IDataView input) => new PcaTransformer(_host, input, _columns);
 

src/Microsoft.ML.Transforms/GcnTransform.cs

@@ -191,6 +191,9 @@ private static VersionInfo GetVersionInfo()
         // REVIEW: should this be an argument instead?
         private const float MinScale = 1e-8f;
 
+        /// <summary>
+        /// The objects describing how the transformation is applied on the input data.
+        /// </summary>
         public IReadOnlyCollection<LpNormalizingEstimatorBase.ColumnInfoBase> Columns => _columns.AsReadOnly();
         private readonly LpNormalizingEstimatorBase.ColumnInfoBase[] _columns;
 
@@ -660,13 +663,28 @@ public enum NormalizerKind : byte
         /// </summary>
         public abstract class ColumnInfoBase
         {
+            /// <summary>
+            /// Name of the column resulting from the transformation of <see cref="InputColumnName"/>.
+            /// </summary>
             public readonly string Name;
+            /// <summary>
+            /// Name of column to transform. If set to <see langword="null"/>, the value of the <see cref="Name"/> will be used as source.
+            /// </summary>
             public readonly string InputColumnName;
+            /// <summary>
+            /// Subtract mean from each value before normalizing.
+            /// </summary>
             public readonly bool SubtractMean;
+            /// <summary>
+            /// The norm to use to normalize each sample.
+            /// </summary>
             public readonly NormalizerKind NormKind;
+            /// <summary>
+            /// Scale features by this value.
+            /// </summary>
             public readonly float Scale;
 
-            internal ColumnInfoBase(string name, string inputColumnName, bool substractMean, LpNormalizingEstimatorBase.NormalizerKind normalizerKind, float scale)
+            internal ColumnInfoBase(string name, string inputColumnName, bool substractMean, NormalizerKind normalizerKind, float scale)
             {
                 Contracts.CheckNonWhiteSpace(name, nameof(name));
                 Contracts.CheckNonWhiteSpace(inputColumnName, nameof(inputColumnName));
@@ -692,14 +710,14 @@ internal ColumnInfoBase(ModelLoadContext ctx, string name, string inputColumnNam
                 // Float: Scale
                 SubtractMean = ctx.Reader.ReadBoolByte();
                 byte normKindVal = ctx.Reader.ReadByte();
-                Contracts.CheckDecode(Enum.IsDefined(typeof(LpNormalizingEstimatorBase.NormalizerKind), normKindVal));
-                NormKind = (LpNormalizingEstimatorBase.NormalizerKind)normKindVal;
+                Contracts.CheckDecode(Enum.IsDefined(typeof(NormalizerKind), normKindVal));
+                NormKind = (NormalizerKind)normKindVal;
                 // Note: In early versions, a bool option (useStd) to whether to normalize by StdDev rather than
                 // L2 norm was used. normKind was added in version=verVectorNormalizerSupported.
                 // normKind was defined in a way such that the serialized boolean (0: use StdDev, 1: use L2) is
                 // still valid.
                 Contracts.CheckDecode(normKindSerialized ||
-                        (NormKind == LpNormalizingEstimatorBase.NormalizerKind.L2Norm || NormKind == LpNormalizingEstimatorBase.NormalizerKind.StdDev));
+                        (NormKind == NormalizerKind.L2Norm || NormKind == NormalizerKind.StdDev));
                 Scale = ctx.Reader.ReadFloat();
                 Contracts.CheckDecode(0 < Scale && Scale < float.PositiveInfinity);
             }
@@ -776,17 +794,17 @@ public override SchemaShape GetOutputSchema(SchemaShape inputSchema)
     }
 
     /// <summary>
-    /// Lp Normalizing estimator allow you take columns and normalize them individually by rescaling them to unit norm.
+    /// Lp Normalizing estimator takes columns and normalizes them individually by rescaling them to unit norm.
     /// </summary>
     public sealed class LpNormalizingEstimator : LpNormalizingEstimatorBase
     {
         /// <summary>
-        /// Describes how the transformer handles one LpNorm column pair.
+        /// Describes how the transformer handles one column pair.
         /// </summary>
         public sealed class LpNormColumnInfo : ColumnInfoBase
         {
             /// <summary>
-            /// Describes how the transformer handles one LpNorm column pair.
+            /// Describes how the transformer handles one column pair.
             /// </summary>
             /// <param name="name">Name of the column resulting from the transformation of <paramref name="inputColumnName"/>.</param>
             /// <param name="inputColumnName">Name of column to transform. If set to <see langword="null"/>, the value of the <paramref name="name"/> will be used as source.</param>
@@ -832,7 +850,7 @@ internal LpNormalizingEstimator(IHostEnvironment env, params LpNormColumnInfo[]
     }
 
     /// <summary>
-    /// Global contrast normalizing estimator allow you take columns and performs global constrast normalization on them.
+    /// Global contrast normalizing estimator takes columns and performs global constrast normalization.
     /// </summary>
     public sealed class GlobalContrastNormalizingEstimator : LpNormalizingEstimatorBase
     {

src/Microsoft.ML.Transforms/RandomFourierFeaturizing.cs

@@ -31,6 +31,9 @@
 
 namespace Microsoft.ML.Transforms.Projections
 {
+    /// <summary>
+    /// Maps vector columns to a random low-dimensional feature space.
+    /// </summary>
     public sealed class RandomFourierFeaturizingTransformer : OneToOneTransformerBase
     {
         internal sealed class Options
@@ -609,23 +612,45 @@ private void TransformFeatures(in VBuffer<float> src, ref VBuffer<float> dst, Tr
     }
 
     /// <summary>
-    /// Estimator which takes set of vector columns and maps its input to a random low-dimensional feature space.
+    /// Maps vector columns to a random low-dimensional feature space.
     /// </summary>
     public sealed class RandomFourierFeaturizingEstimator : IEstimator<RandomFourierFeaturizingTransformer>
     {
-        public static class Defaults
+        [BestFriend]
+        internal static class Defaults
         {
             public const int NewDim = 1000;
             public const bool UseSin = false;
         }
 
+        /// <summary>
+        /// Describes how the transformer handles one Gcn column pair.
+        /// </summary>
         public sealed class ColumnInfo
         {
+            /// <summary>
+            /// Name of the column resulting from the transformation of <see cref="InputColumnName"/>.
+            /// </summary>
             public readonly string Name;
+            /// <summary>
+            /// Name of the column to transform. If set to <see langword="null"/>, the value of the <see cref="Name"/> will be used as source.
+            /// </summary>
             public readonly string InputColumnName;
+            /// <summary>
+            /// Which fourier generator to use.
+            /// </summary>
             public readonly IComponentFactory<float, IFourierDistributionSampler> Generator;
+            /// <summary>
+            /// The number of random Fourier features to create.
+            /// </summary>
             public readonly int NewDim;
+            /// <summary>
+            /// Create two features for every random Fourier frequency? (one for cos and one for sin).
+            /// </summary>
             public readonly bool UseSin;
+            /// <summary>
+            /// The seed of the random number generator for generating the new features (if unspecified, the global random is used).
+            /// </summary>
             public readonly int? Seed;
 
             /// <summary>
@@ -636,7 +661,7 @@ public sealed class ColumnInfo
             /// <param name="useSin">Create two features for every random Fourier frequency? (one for cos and one for sin).</param>
             /// <param name="inputColumnName">Name of column to transform. </param>
             /// <param name="generator">Which fourier generator to use.</param>
-            /// <param name="seed">The seed of the random number generator for generating the new features (if unspecified, the global random is used.</param>
+            /// <param name="seed">The seed of the random number generator for generating the new features (if unspecified, the global random is used).</param>
             public ColumnInfo(string name, int newDim, bool useSin, string inputColumnName = null, IComponentFactory<float, IFourierDistributionSampler> generator = null, int? seed = null)
             {
                 Contracts.CheckUserArg(newDim > 0, nameof(newDim), "must be positive.");
@@ -673,7 +698,7 @@ internal RandomFourierFeaturizingEstimator(IHostEnvironment env, params ColumnIn
         }
 
         /// <summary>
-        /// Train and return a transformer.
+        /// Trains and returns a <see cref="RandomFourierFeaturizingTransformer"/>.
         /// </summary>
         public RandomFourierFeaturizingTransformer Fit(IDataView input) => new RandomFourierFeaturizingTransformer(_host, input, _columns);
 

src/Microsoft.ML.Transforms/Text/StopWordsRemovingTransformer.cs

@@ -809,6 +809,9 @@ private void LoadStopWords(IChannel ch, ReadOnlyMemory<char> stopwords, string d
             }
         }
 
+        /// <summary>
+        /// The names of the input output column pairs on which this transformation is applied.
+        /// </summary>
         public IReadOnlyCollection<(string outputColumnName, string inputColumnName)> Columns => ColumnPairs.AsReadOnly();
 
         /// <summary>