Normalize documentation #3244

Ivanidzo4ka · 2019-04-08T21:16:21Z

towards #1209

codecov · 2019-04-08T21:57:18Z

Codecov Report

Merging #3244 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3244      +/-   ##
==========================================
- Coverage   72.65%   72.64%   -0.01%     
==========================================
  Files         807      807              
  Lines      145191   145191              
  Branches    16223    16223              
==========================================
- Hits       105485   105480       -5     
- Misses      35290    35293       +3     
- Partials     4416     4418       +2

Flag	Coverage Δ
#Debug	`72.64% <ø> (-0.01%)`	⬇️
#production	`68.17% <ø> (-0.01%)`	⬇️
#test	`88.97% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
src/Microsoft.ML.Transforms/NormalizerCatalog.cs	`84.78% <ø> (ø)`	⬆️
src/Microsoft.ML.Maml/MAML.cs	`24.75% <0%> (-1.46%)`	⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs	`84.7% <0%> (-0.21%)`	⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs	`86.1% <0%> (-0.16%)`	⬇️

artidoro · 2019-04-09T00:16:16Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs

+            foreach (var row in columnFixZero)
+                Console.WriteLine(string.Join(", ", row.Select(x => x.ToString("f4"))));
+            // 1.0000, 0.0000, 1.0000, 0.0000
+            // 0.5000, 0.5000, 0.0000, 0.5000


nit: could you add (here and in other files)
// Expected output: #Resolved

I suspect expected values are incorrect: input: 2, 2, 2, 0 minmaxnormalized returns 1,1,1,0 or am I missing something?

artidoro · 2019-04-09T00:23:38Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs

+            var transformParams = (normalizeTransform.GetNormalizerModelParameters(0) as Microsoft.ML.Transforms.NormalizingTransformer.BinNormalizerModelParameters<ImmutableArray<float>>);
+            Console.WriteLine($"Values for slot 0 would be transfromed by applying y = (Index(x) / {transformParams.Density[0]}) - {(transformParams.Offset.Length == 0 ? 0 : transformParams.Offset[0])}");
+            Console.WriteLine("Where Index(x) is index of bin to which x belongs");
+            Console.WriteLine($"Bins upper borders are: {string.Join(" ", transformParams.UpperBounds[0])}");


borders [](start = 43, length = 7)

Maybe would suggest to use "bounds" instead of "borders". #Resolved

artidoro · 2019-04-09T00:28:20Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs

+            // 0.0000, -0.5000, 0.0000, 1.0000
+            // 0.0000, -0.5000, 0.0000, 1.0000
+
+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for function.


for function. [](start = 118, length = 13)

.... as parameter to GetNormalizerModelParameters. #Resolved

artidoro · 2019-04-09T00:28:52Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs

+            // 0.0000, -0.5000, 0.0000, 1.0000
+
+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for function.
+            // If case if we have multiple column transformations we need to pass index of InputOutputColumnPair.


If case [](start = 15, length = 8)

Remove "If case", ere and in other files. #Resolved

artidoro · 2019-04-09T00:32:44Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeMinMax.cs

+            // If case if we have multiple column transformations we need to pass index of InputOutputColumnPair.
+            var transformParams = (normalizeTransform.GetNormalizerModelParameters(0) as Microsoft.ML.Transforms.NormalizingTransformer.AffineNormalizerModelParameters<ImmutableArray<float>>);
+            Console.WriteLine($"Values for slot 1 would be transfromed by applying y= (x - ({(transformParams.Offset.Length == 0 ? 0 : transformParams.Offset[1])})) * {transformParams.Scale[1]}");
+            // Values for slot 1 would be transfromed by applying y= (x - (-1)) * 0.3333333


y= [](start = 66, length = 3)

could you add a space after "y"? #Resolved

artidoro · 2019-04-09T00:37:10Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeMeanVariance.cs

+            var normalize = mlContext.Transforms.NormalizeMeanVariance("Features", useCdf: true);
+
+            // NormalizeMeanVariance normalizes the data based on the computed mean and variance of the data.
+            var normalizeNoCdf = mlContext.Transforms.NormalizeMeanVariance("Features", useCdf: false);


I would not repeat what NormalizeMeanVariance does.

artidoro · 2019-04-09T00:37:39Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+
+            // NormalizeBinning normalizes the data by constructing equidensity bins and produce output based on 
+            // to which bin original value belong but make sure zero values would remain zero after normalization.
+            // Helps preserve sparsity.


Maybe I would not repeat this explanation. Here and also in other samples.

artidoro · 2019-04-09T00:38:51Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs

+            Console.WriteLine("Where Index(x) is index of bin to which x belongs");
+            Console.WriteLine($"Bins upper borders are: {string.Join(" ", fixZeroParams.UpperBounds[1])}");
+            // Values for slot 1 would be transfromed by applying y = (Index(x) / 2) - 0.5
+            // Where Index(x) is index of bin to which x belongs


Where Index(x) is index of bin to which x belongs [](start = 15, length = 49)

// Where Index(x) is the index of the bin to which x belongs" #Resolved

zeahmed · 2019-04-09T22:58:15Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            var normalize = mlContext.Transforms.NormalizeBinning("Features", maximumBinCount: 4, fixZero: false);
+
+            // NormalizeBinning normalizes the data by constructing equidensity bins and produce output based on 
+            // to which bin original value belong but make sure zero values would remain zero after normalization.


duplicate comment...see line 26-27 #Closed

ohh I see you are making another estimator here...

In reply to: 273736107 [](ancestors = 273736107)

zeahmed · 2019-04-09T23:01:03Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+
+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for GetNormalizerModelParameters.
+            // If we have multiple column transformations we need to pass index of InputOutputColumnPair.
+            var transformParams = (normalizeTransform.GetNormalizerModelParameters(0) as Microsoft.ML.Transforms.NormalizingTransformer.BinNormalizerModelParameters<ImmutableArray<float>>);


Microsoft.ML.Transforms [](start = 89, length = 23)

Can we make it a using statement? This line is getting very long. #Resolved

zeahmed · 2019-04-09T23:14:17Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeLogMeanVariance.cs

+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for GetNormalizerModelParameters.
+            // If we have multiple column transformations we need to pass index of InputOutputColumnPair.
+            var transformParams = normalizeTransform.GetNormalizerModelParameters(0) as CdfNormalizerModelParameters<ImmutableArray<float>>;
+            Console.WriteLine($"Values for slot 1 would be transfromed by applying y = 0.5* (1 + ERF((Math.Log(x)- {transformParams.Mean[1]}) / ({transformParams.StandardDeviation[1]} * sqrt(2)))" );


slot 1 [](start = 43, length = 6)

What are slot 0 and slot 1? In this particular sample this one and below are both slot 1? #Resolved

rephrased

In reply to: 273739363 [](ancestors = 273739363)

zeahmed · 2019-04-09T23:15:32Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeMinMax.cs

+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for GetNormalizerModelParameters.
+            // If we have multiple column transformations we need to pass index of InputOutputColumnPair.
+            var transformParams = normalizeTransform.GetNormalizerModelParameters(0) as AffineNormalizerModelParameters<ImmutableArray<float>>;
+            Console.WriteLine($"Values for slot 1 would be transfromed by applying y = (x - ({(transformParams.Offset.Length == 0 ? 0 : transformParams.Offset[1])})) * {transformParams.Scale[1]}");


slot 1 [](start = 43, length = 6)

see my comments in other sample. #Resolved

sfilipi · 2019-04-10T04:32:13Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+
+namespace Samples.Dynamic
+{
+    public class NormalizeBinning


NormalizeBinning [](start = 17, length = 16)

one line comment about what this does.

I don't see that pattern in our documentation

In reply to: 273786125 [](ancestors = 273786125)

sfilipi · 2019-04-10T04:34:15Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            // NormalizeBinning normalizes the data by constructing equidensity bins and produce output based on 
+            // to which bin original value belong but make sure zero values would remain zero after normalization.
+            // Helps preserve sparsity.
+            var normalizeFixZero = mlContext.Transforms.NormalizeBinning("Features", maximumBinCount: 4, fixZero: true);


fixZero: true [](start = 105, length = 13)

love the variation on the parameter! #WontFix

sfilipi · 2019-04-10T04:35:21Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            // Expected output:
+            //  1.0000, 0.6667, 1.0000, 0.0000
+            //  0.6667, 1.0000, 0.6667, 0.0000
+            //  0.3333, 0.3333, 0.3333, 0.0000


0.3333 [](start = 24, length = 6)

I'd emphasize this value.

I don't get your comment.

In reply to: 273786583 [](ancestors = 273786583)

sfilipi · 2019-04-10T04:36:41Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            //  0.3333, 0.0000, 0.3333, 0.0000
+            //  0.0000, -0.3333, 0.0000, 1.0000
+
+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for GetNormalizerModelParameters.


we need to pass 0, the index of this column in the dataview, as parameter for..

No

In reply to: 273786759 [](ancestors = 273786759)

sfilipi · 2019-04-10T04:37:05Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            //  0.0000, -0.3333, 0.0000, 1.0000
+
+            // Let's get transformation parameters. Since we work with only one column we need to pass 0 as parameter for GetNormalizerModelParameters.
+            // If we have multiple column transformations we need to pass index of InputOutputColumnPair.


multiple column transformations [](start = 26, length = 31)

multiple columns #Resolved

sfilipi

sfilipi · 2019-04-10T05:26:31Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeMinMax.cs

+            var mlContext = new MLContext();
+            var samples = new List<DataPoint>()
+            {
+                new DataPoint(){ Features = new float[4] { 1, 1, 3, 0} },


maybe no need to have two columns with the same exact values.

sfilipi · 2019-04-10T05:31:18Z

src/Microsoft.ML.Transforms/NormalizerCatalog.cs

+        /// [!code-csharp[NormalizeMinMax](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeMinMax.cs)]
+        /// ]]>
+        /// </format>
+        /// </example>
        public static NormalizingEstimator NormalizeMinMax(this TransformsCatalog catalog, InputOutputColumnPair[] columns,


NormalizeMinMax [](start = 43, length = 15)

you're not invoking this API. I would not put the example here. #Resolved

sfilipi · 2019-04-10T05:31:45Z

src/Microsoft.ML.Transforms/NormalizerCatalog.cs

@@ -137,7 +165,7 @@ public static class NormalizationCatalog
        /// <example>
        /// <format type="text/markdown">
        /// <![CDATA[
-        /// [!code-csharp[Normalize](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Normalizer.cs)]
+        /// [!code-csharp[NormalizeLogMeanVariance](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeLogMeanVariance.cs)]


[!code-csharp[NormalizeLogMeanVariance] [](start = 12, length = 39)

you're not invoking this API. I would not put the example here. #Resolved

sfilipi · 2019-04-10T05:32:10Z

src/Microsoft.ML.Transforms/NormalizerCatalog.cs

@@ -175,6 +210,13 @@ public static class NormalizationCatalog
        /// <param name="maximumExampleCount">Maximum number of examples used to train the normalizer.</param>
        /// <param name="fixZero">Whether to map zero to zero, preserving sparsity.</param>
        /// <param name="maximumBinCount">Maximum number of bins (power of 2 recommended).</param>
+        /// <example>


you're not invoking this API. I would not put the example here. #Resolved

sfilipi · 2019-04-10T05:32:28Z

src/Microsoft.ML.Transforms/NormalizerCatalog.cs

+        /// [!code-csharp[NormalizeBinning](~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeSupervisedBinning.cs)]
+        /// ]]>
+        /// </format>
+        /// </example>


[](start = 21, length = 1)

you're not invoking this API. I would not put the example here. #Resolved

shmoradims · 2019-04-10T16:44:57Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+using System.Linq;
+using Microsoft.ML;
+using Microsoft.ML.Data;
+using static Microsoft.ML.Transforms.NormalizingTransformer;


do we need this? #ByDesign

Yes, otherwise I need to write Microsoft.ML.Transforms.NormalizingTransformer.BinNormalizerModelParameters which takes too much space

In reply to: 274057369 [](ancestors = 274057369)

shmoradims · 2019-04-10T16:48:20Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            // If we have multiple column transformations we need to pass index of InputOutputColumnPair.
+            var transformParams = normalizeTransform.GetNormalizerModelParameters(0) as BinNormalizerModelParameters<ImmutableArray<float>>;
+            Console.WriteLine($"Values for slot 0 would be transfromed by applying y = (Index(x) / {transformParams.Density[0]}) - {(transformParams.Offset.Length == 0 ? 0 : transformParams.Offset[0])}");
+            Console.WriteLine("Where Index(x) is the index of the bin to which x belongs");


please break super-long lines into two lines #Resolved

shmoradims · 2019-04-10T16:49:55Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeLogMeanVariance.cs

+using System.Linq;
+using Microsoft.ML;
+using Microsoft.ML.Data;
+using static Microsoft.ML.Transforms.NormalizingTransformer;


sing static Microsoft.ML.Transforms.NormalizingTransformer; [](start = 1, length = 59)

ditto #Resolved

shmoradims

shmoradims · 2019-04-12T21:44:50Z

docs/samples/Microsoft.ML.Samples/Dynamic/Transforms/NormalizeBinning.cs

+            var normalize = mlContext.Transforms.NormalizeBinning("Features", maximumBinCount: 4, fixZero: false);
+
+            // NormalizeBinning normalizes the data by constructing equidensity bins and produce output based on 
+            // to which bin original value belong but make sure zero values would remain zero after normalization.


please move the details to similar to your other PR

Normalize documentation

3eca87e

Ivanidzo4ka requested review from codemzs, wschin, artidoro, shmoradims and sfilipi April 8, 2019 21:16

artidoro reviewed Apr 9, 2019

View reviewed changes

address some comments

aaec479

zeahmed reviewed Apr 9, 2019

View reviewed changes

Expected output shited by one space

611172a

zeahmed reviewed Apr 9, 2019

View reviewed changes

Ivan Matantsev added 4 commits April 9, 2019 16:05

using static Microsoft.ML.Transforms.NormalizingTransformer

9cffe3a

revert program.cs

5d2da69

revert one more time

215a4ab

one more time

66fe40a

zeahmed reviewed Apr 9, 2019

View reviewed changes

merge with master

6217b99

sfilipi reviewed Apr 10, 2019

View reviewed changes

sfilipi approved these changes Apr 10, 2019

View reviewed changes

sfilipi reviewed Apr 10, 2019

View reviewed changes

shmoradims reviewed Apr 10, 2019

View reviewed changes

Ivan Matantsev added 4 commits April 11, 2019 12:45

Merge branch 'master' into Ivanidze/NormalizeDocumentation

6bda83d

Address comments

ed811fc

plurals

43ca3e8

Merge branch 'master' into Ivanidze/NormalizeDocumentation

b0b639c

shmoradims approved these changes Apr 12, 2019

View reviewed changes

shmoradims reviewed Apr 12, 2019

View reviewed changes

Ivanidzo4ka merged commit 96b3b2a into dotnet:master Apr 15, 2019

Ivanidzo4ka mentioned this pull request Apr 15, 2019

Cherry pick Projection documentation and Normalize changes to 1.0 #3344

Closed

sfilipi mentioned this pull request Apr 16, 2019

API reference - Samples for Transforms #1209

Closed

dotnet locked as resolved and limited conversation to collaborators Mar 23, 2022

Normalize documentation #3244

Normalize documentation #3244

Conversation

Ivanidzo4ka commented Apr 8, 2019

codecov bot commented Apr 8, 2019 • edited

Codecov Report

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artidoro Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

zeahmed Apr 9, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

zeahmed Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeahmed Apr 9, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

sfilipi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

sfilipi Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

shmoradims Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shmoradims Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

shmoradims Apr 10, 2019 • edited by Ivanidzo4ka

Choose a reason for hiding this comment

shmoradims left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 8, 2019 •

edited

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

artidoro Apr 9, 2019 •

edited by Ivanidzo4ka

zeahmed Apr 9, 2019 •

edited

zeahmed Apr 9, 2019 •

edited by Ivanidzo4ka

zeahmed Apr 9, 2019 •

edited by Ivanidzo4ka

zeahmed Apr 9, 2019 •

edited by Ivanidzo4ka

sfilipi Apr 10, 2019 •

edited

sfilipi Apr 10, 2019 •

edited by Ivanidzo4ka

sfilipi Apr 10, 2019 •

edited by Ivanidzo4ka

sfilipi Apr 10, 2019 •

edited by Ivanidzo4ka

sfilipi Apr 10, 2019 •

edited by Ivanidzo4ka

sfilipi Apr 10, 2019 •

edited by Ivanidzo4ka

shmoradims Apr 10, 2019 •

edited by Ivanidzo4ka

shmoradims Apr 10, 2019 •

edited by Ivanidzo4ka

shmoradims Apr 10, 2019 •

edited by Ivanidzo4ka