Add epsilon and ddof (delta degrees of freedom) arguments to Normalize. #1964

mzient · 2020-05-14T17:46:40Z

Signed-off-by: Michał Zientkiewicz mzient@gmail.com

Why we need this PR?

Pick one, remove the rest

It adds new feature (degrees of freedom) needed to maintain compatibility with parameters of stddev in PyTorch and in numpy
It adds regularizing term to normalization (added to variance)

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

What solution was applied:
- Add epsilon added to variance
- And ddof (delta degrees of freedom), subtracted from variance's denominator
- Improve normalization precision on CPU (Newton-Raphson step)
- Add ddof and epsilon to tests
Affected modules and functionalities:
- Normalize operator
Key points relevant for the review:
- ?
Validation and testing:
- End-to-end python test
Documentation (including examples):
- New arguments documented in schema

JIRA TASK: N/A

mzient · 2020-05-14T17:57:04Z

!build

dali-automaton · 2020-05-14T18:00:33Z

CI MESSAGE: [1322135]: BUILD STARTED

Improve normalization precision on CPU. Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

JanuszL · 2020-05-14T18:35:00Z

dali/test/python/test_operator_normalize.py

-        var = ((x - mean).astype(np.float)**2).mean(axis = axes, keepdims = True)
-        stddev = np.sqrt(var)
+
+    if stddev is None:


Maybe you can just use numpy.std at least for some case?

It's going to be a bit artificial, but I can give it a shot.

I'm asking to validate if or definition of ddof matches the one from numpy. But it is just a suggestion.

dali-automaton · 2020-05-14T19:28:18Z

CI MESSAGE: [1322135]: BUILD FAILED

dali-automaton · 2020-05-14T21:22:18Z

CI MESSAGE: [1322135]: BUILD PASSED

Apply epsilon to explicit scalar stddev. Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

mzient · 2020-05-14T21:34:19Z

!build

dali-automaton · 2020-05-15T00:17:51Z

CI MESSAGE: [1322839]: BUILD STARTED

dali-automaton · 2020-05-15T01:47:03Z

CI MESSAGE: [1322839]: BUILD PASSED

klecki · 2020-05-15T15:30:47Z

dali/operators/math/normalize/normalize.cc

-    scalar_inv_stddev = scale_ / spec_.GetArgument<float>("stddev");
+    float scalar_stddev = spec_.GetArgument<float>("stddev");
+    if (epsilon_)
+      scalar_inv_stddev = scale_ * rsqrt(scalar_stddev*scalar_stddev + epsilon_);


It took me too long to connect the fact that we're adding epsilon to Variance and we're handling the Standard deviation here, hence the square and square root. Can you maybe add a comment that would make it more obvious here, the docs for epsilon is quite distant.

klecki · 2020-05-15T15:31:20Z

dali/operators/math/normalize/normalize_utils.h

+    int64_t v = volume(inv.shape.tensor_shape_span(i));
+    if (epsilon) {
+      for (int64_t j = 0; j < v; j++) {
+        inv.data[i][j] = scale * rsqrt(stddev.data[i][j] * stddev.data[i][j] + epsilon);
+      }
+    } else {
+      for (int64_t j = 0; j < v; j++) {
+        inv.data[i][j] = stddev.data[i][j] ? scale / stddev.data[i][j] : 0;
+      }
    }


Added Doxygen.

klecki · 2020-05-15T15:32:01Z

dali/operators/math/normalize/normalize.cc

+  float scale = scale_;
+  if (v > degrees_of_freedom_) {
+    rdiv = static_cast<float>(1.0 / (v - degrees_of_freedom_));
+  } else {


Shouldn't we error in such cases?

Numpy will give you infinite stddev - so dividing by it will produce zero - I'm sort-of emulating that.

klecki · 2020-05-15T15:34:10Z

dali/operators/math/normalize/normalize_utils.h

+  //
+  // rsqrt needs an extra step of Newton-Raphson refinement:
+  // rough = approx_rsqrt(x)
+  // precise = rough * (3 + x*y*y) * 0.5


Sorry, but what is y?

As I'm already nitpicking there is 3 + xyy and in the code it's actually 3-xyy.

I'll fix that.

klecki · 2020-05-15T15:36:27Z

dali/operators/math/normalize/normalize_utils.h

+  // Vectorized version of the loop below
+
+  // We calculate the following:
+  // mul * rsqrt(data[i] + eps)


Suggested change

// mul * rsqrt(data[i] + eps)

// mul * rsqrt(data[i] * rdiv + eps)

Maybe that should go into @brief section of this function?

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

mzient · 2020-05-18T09:16:10Z

!build

dali-automaton · 2020-05-18T09:20:36Z

CI MESSAGE: [1328752]: BUILD STARTED

dali-automaton · 2020-05-18T11:02:45Z

CI MESSAGE: [1328752]: BUILD PASSED

mzient requested a review from a team May 14, 2020 17:46

mzient force-pushed the NormalizeRegularizingTerm branch from 60dafbf to 34f5f10 Compare May 14, 2020 17:51

mzient force-pushed the NormalizeRegularizingTerm branch 2 times, most recently from a1bfcd1 to 57bfed8 Compare May 14, 2020 18:00

Add epsilon and ddof (delta degrees of freedom) arguments to Normalize.

18dc925

Improve normalization precision on CPU. Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

mzient force-pushed the NormalizeRegularizingTerm branch from 57bfed8 to 18dc925 Compare May 14, 2020 18:03

JanuszL reviewed May 14, 2020

View reviewed changes

JanuszL approved these changes May 14, 2020

View reviewed changes

Use numpy.std in some tests.

7da97ae

Apply epsilon to explicit scalar stddev. Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

mzient force-pushed the NormalizeRegularizingTerm branch from ffff42f to 7da97ae Compare May 14, 2020 21:32

klecki reviewed May 15, 2020

View reviewed changes

Fix comments.

516cd7d

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>

klecki approved these changes May 18, 2020

View reviewed changes

mzient merged commit 08cfe99 into NVIDIA:master May 18, 2020

mzient deleted the NormalizeRegularizingTerm branch May 18, 2020 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add epsilon and ddof (delta degrees of freedom) arguments to Normalize. #1964

Add epsilon and ddof (delta degrees of freedom) arguments to Normalize. #1964

mzient commented May 14, 2020

mzient commented May 14, 2020

dali-automaton commented May 14, 2020

JanuszL May 14, 2020

mzient May 14, 2020

JanuszL May 14, 2020

mzient May 14, 2020

dali-automaton commented May 14, 2020

dali-automaton commented May 14, 2020

mzient commented May 14, 2020

dali-automaton commented May 15, 2020

dali-automaton commented May 15, 2020

klecki May 15, 2020

mzient May 18, 2020

klecki May 15, 2020

mzient May 18, 2020

klecki May 15, 2020

mzient May 18, 2020

klecki May 15, 2020

klecki May 15, 2020

mzient May 18, 2020

klecki May 15, 2020

mzient commented May 18, 2020

dali-automaton commented May 18, 2020

dali-automaton commented May 18, 2020

	// mul * rsqrt(data[i] + eps)
	// mul * rsqrt(data[i] * rdiv + eps)

Add epsilon and ddof (delta degrees of freedom) arguments to Normalize. #1964

Add epsilon and ddof (delta degrees of freedom) arguments to Normalize. #1964

Conversation

mzient commented May 14, 2020

Why we need this PR?

What happened in this PR?

mzient commented May 14, 2020

dali-automaton commented May 14, 2020

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

dali-automaton commented May 14, 2020

dali-automaton commented May 14, 2020

mzient commented May 14, 2020

dali-automaton commented May 15, 2020

dali-automaton commented May 15, 2020

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

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

mzient commented May 18, 2020

dali-automaton commented May 18, 2020

dali-automaton commented May 18, 2020