From afd50e2bb84b4291abd333b672da7e10ebf5ec8b Mon Sep 17 00:00:00 2001
From: James <j.c.hollyer@gmail.com>
Date: Fri, 17 Mar 2023 12:32:54 -0700
Subject: [PATCH] Bug fix: Use Standard ticks when dealing with scientific
 notation (#6247)

* Motivation for features / changes
When the differences between the max and min points on an axis are very
small we use some special logic to make the axis tick marks more
readable. However, this logic assumes the numbers to be decimal and
gives NaN values when the numbers take on scientific notation.
Fortunately more times scientific notation is used the number become
small enough to use regular tick marks. This change simply reverts back
to standard tick marks whenever javascript starts using scientific
notation.

* Screenshots of UI changes
Before:
<img width="90" alt="Screenshot 2023-03-16 at 9 37 26 PM"
src="https://user-images.githubusercontent.com/8672809/225814036-3b622a9c-bc63-47c4-ad09-6eea1b5cf727.png">

After:
<img width="72" alt="Screenshot 2023-03-16 at 9 39 40 PM"
src="https://user-images.githubusercontent.com/8672809/225814092-48faa8e8-11c8-4ba5-a27f-648dacae75d7.png">
---
 .../sub_view/line_chart_axis_utils.ts         | 22 +++++++++++++++
 .../sub_view/line_chart_axis_utils_test.ts    | 28 ++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts
index 75ddd0d587..8b74268570 100644
--- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts
+++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils.ts
@@ -102,6 +102,17 @@ function getTicksForLinearScale(
 
   const minorTickVals = scale.ticks([low, high], maxMinorTickCount);
   const majorTickVals = scale.ticks([low, high], 2);
+
+  // If the numbers are small enough that javascript starts using scientific
+  // notation the logic here does not work. Also, those numbers normally show up
+  // well using the standard ticks.
+  if (
+    containsScientificNotation(minorTickVals) ||
+    containsScientificNotation(majorTickVals)
+  ) {
+    return getStandardTicks(scale, formatter, maxMinorTickCount, lowAndHigh);
+  }
+
   const minor: MinorTick[] = [];
 
   let numFractionalToKeep = getNumLeadingZerosInFractional(diff);
@@ -224,9 +235,20 @@ function filterTicksByVisibility(
   });
 }
 
+function containsScientificNotation(values: number[]): boolean {
+  for (const value of values) {
+    if (String(value).includes('e')) {
+      return true;
+    }
+  }
+  return false;
+}
+
 export const AxisUtils = {
   getStandardTicks,
   getTicksForTemporalScale,
   getTicksForLinearScale,
   filterTicksByVisibility,
 };
+
+export const TEST_ONLY = {containsScientificNotation};
diff --git a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts
index 2f64f7fc30..b025410645 100644
--- a/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts
+++ b/tensorboard/webapp/widgets/line_chart_v2/sub_view/line_chart_axis_utils_test.ts
@@ -14,7 +14,7 @@ limitations under the License.
 ==============================================================================*/
 
 import {createScale, LinearScale, ScaleType, TemporalScale} from '../lib/scale';
-import {AxisUtils} from './line_chart_axis_utils';
+import {AxisUtils, TEST_ONLY} from './line_chart_axis_utils';
 
 describe('line_chart_v2/sub_view/axis_utils test', () => {
   describe('#getStandardTicks', () => {
@@ -363,6 +363,32 @@ describe('line_chart_v2/sub_view/axis_utils test', () => {
           {value: -0.00001, tickFormattedString: '…0'},
         ]);
       });
+      it('returns no major ticks for numbers which are transated to scientific notation', () => {
+        const {major, minor} = AxisUtils.getTicksForLinearScale(
+          scale,
+          scale.defaultFormatter,
+          5,
+          [0.0000000004, 0.000000009]
+        );
+
+        expect(major).toEqual([]);
+        expect(minor).toEqual([
+          {value: 2e-9, tickFormattedString: '2e-9'},
+          {value: 4e-9, tickFormattedString: '4e-9'},
+          {value: 6e-9, tickFormattedString: '6e-9'},
+          {value: 8e-9, tickFormattedString: '8e-9'},
+        ]);
+      });
+    });
+  });
+
+  describe('#containsScientificNotation', () => {
+    it('returns true if the array contains scientific notation', () => {
+      expect(TEST_ONLY.containsScientificNotation([1, 2e-9, 2])).toBe(true);
+    });
+
+    it('returns false if the array does not contain scientific notation', () => {
+      expect(TEST_ONLY.containsScientificNotation([1, 4, 2])).toBe(false);
     });
   });