From 7859719d68a530a027d766b983de5fed9f6c9195 Mon Sep 17 00:00:00 2001
From: Yating <ytjing@google.com>
Date: Thu, 4 Nov 2021 18:59:52 -0400
Subject: [PATCH] histogram: make timeseries histogram visualization compatible
 with v3 data format (#5392)

* make timeseries histogram visualization compatible with v3 data format

* improve getBinContribution description (thanks nfelt@!)

* update test for edge case zero-width bin scenarios

* remove duplicate test case

* update doc

* improve test
---
 .../widgets/histogram/histogram_util.ts       | 22 +++--
 .../widgets/histogram/histogram_util_test.ts  | 86 +++++++------------
 2 files changed, 50 insertions(+), 58 deletions(-)

diff --git a/tensorboard/webapp/widgets/histogram/histogram_util.ts b/tensorboard/webapp/widgets/histogram/histogram_util.ts
index 795dae035c3..fafaf61f9a7 100644
--- a/tensorboard/webapp/widgets/histogram/histogram_util.ts
+++ b/tensorboard/webapp/widgets/histogram/histogram_util.ts
@@ -148,9 +148,21 @@ function rebuildBins(bins: Bin[], range: Range, binCount: number): Bin[] {
 }
 
 /**
- * Computes how much of the input bin's 'y' counts should be allocated to a new
- * range. For 0 width input bins, the allocation may be split in half across 2
- * bins.
+ * Computes how much of the input bin's 'y' counts should be allocated to this output bin.
+ *
+ * Where both bins have non-zero width, this is computed by multiplying the input y value by
+ * the ratio of the width-wise overlap in the bins to the total width of the output bin.
+ * (This can be thought of redistributing the overlapping "area" of the bar in the input
+ * histogram across the full width of the output bin.)
+ *
+ * When the input bin has zero width (the output bin cannot have zero width by construction),
+ * we instead have to consider several cases depending on the open/closed-ness of the
+ * underlying intervals. If the zero width input bin has y value 0, the contribution is
+ * always 0. Otherwise, if zero width input bin has y value greater than 0, it must represent
+ * the closed interval [x, x]. In this case, it contributes the full value of y if and only
+ * if the output bin's interval contains x. This interval is the closed-open interval
+ * [resultLeft, resultRight), except if resultHasRightNeighbor is false, in which case it's
+ * the closed interval [resultLeft, resultRight].
  */
 function getBinContribution(
   bin: Bin,
@@ -165,8 +177,8 @@ function getBinContribution(
   }
 
   if (bin.dx === 0) {
-    if (resultHasRightNeighbor && binRight === resultRight) {
-      return {curr: 0.5 * bin.y, next: 0.5 * bin.y};
+    if (resultHasRightNeighbor && binRight >= resultRight) {
+      return {curr: 0, next: bin.y};
     }
     return {curr: bin.y, next: 0};
   }
diff --git a/tensorboard/webapp/widgets/histogram/histogram_util_test.ts b/tensorboard/webapp/widgets/histogram/histogram_util_test.ts
index 3f96406520e..8aee2f66dfb 100644
--- a/tensorboard/webapp/widgets/histogram/histogram_util_test.ts
+++ b/tensorboard/webapp/widgets/histogram/histogram_util_test.ts
@@ -260,15 +260,15 @@ describe('histogram util', () => {
         ]);
       });
 
-      it('redistributes 0 width bin evenly over edges of result bins', () => {
+      it('redistributes bin counts where the last bin has zero width', () => {
         expect(
           histogramsToBins(
             buildNormalizedHistograms(
               [
                 binsToHistogram([
-                  {x: 0, dx: 1, y: 0},
-                  {x: 5, dx: 0, y: 200},
-                  {x: 9, dx: 1, y: 0},
+                  {x: 0, dx: 1, y: 10},
+                  {x: 1, dx: 1, y: 10},
+                  {x: 2, dx: 0, y: 10},
                 ]),
               ],
               2
@@ -276,29 +276,8 @@ describe('histogram util', () => {
           )
         ).toEqual([
           [
-            {x: 0, dx: 5, y: 100},
-            {x: 5, dx: 5, y: 100},
-          ],
-        ]);
-      });
-
-      it('redistributes 0 width bins at the edges', () => {
-        expect(
-          histogramsToBins(
-            buildNormalizedHistograms(
-              [
-                binsToHistogram([
-                  {x: 0, dx: 0, y: 100},
-                  {x: 10, dx: 0, y: 200},
-                ]),
-              ],
-              2
-            )
-          )
-        ).toEqual([
-          [
-            {x: 0, dx: 5, y: 100},
-            {x: 5, dx: 5, y: 200},
+            {x: 0, dx: 1, y: 10},
+            {x: 1, dx: 1, y: 20},
           ],
         ]);
       });
@@ -345,32 +324,6 @@ describe('histogram util', () => {
           )
         ).toEqual([[{x: 0, dx: 10, y: 300}]]);
       });
-
-      it(
-        'produces result bins with full and partial contributions from ' +
-          'multiple 0 width bins',
-        () => {
-          expect(
-            histogramsToBins(
-              buildNormalizedHistograms(
-                [
-                  binsToHistogram([
-                    {x: 0, dx: 1, y: 0},
-                    {x: 5, dx: 0, y: 200},
-                    {x: 10, dx: 0, y: 100},
-                  ]),
-                ],
-                2
-              )
-            )
-          ).toEqual([
-            [
-              {x: 0, dx: 5, y: 100},
-              {x: 5, dx: 5, y: 200},
-            ],
-          ]);
-        }
-      );
     });
 
     describe('multiple histograms', () => {
@@ -426,6 +379,33 @@ describe('histogram util', () => {
           ],
         ]);
       });
+
+      it(
+        'produces result bins from multiple 0 width bins in different ' +
+          'steps',
+        () => {
+          expect(
+            histogramsToBins(
+              buildNormalizedHistograms(
+                [
+                  binsToHistogram([{x: 0, dx: 0, y: 200}]),
+                  binsToHistogram([{x: 1.0, dx: 0, y: 100}]),
+                ],
+                2
+              )
+            )
+          ).toEqual([
+            [
+              {x: 0, dx: 0.5, y: 200},
+              {x: 0.5, dx: 0.5, y: 0},
+            ],
+            [
+              {x: 0, dx: 0.5, y: 0},
+              {x: 0.5, dx: 0.5, y: 100},
+            ],
+          ]);
+        }
+      );
     });
   });
 });