From d8a16a4def882a13e6caa99c0663d37630b3371b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dandelion=20Man=C3=A9?= <decentralion@dandelion.io>
Date: Thu, 30 Aug 2018 19:21:59 -0700
Subject: [PATCH] Better handling of log weights (#736)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit isolates all of the log-weight behavior in the weight
slider. That slider moves in log space, but the numbers printed and
passed around the WeightConfig code are now always in linear-space.

This should reduce confusion in the UI and for developers.

This commit contains two other improvements: (#588)
- Changes the (log space) range on the sliders from ±10 to ±5
- Change the order from slider, weight, name to name, slider, weight, so
that there is more visual separation between the name and the weight.

Test plan: Changes to the weight slider are tested. Changes to the
WeightConfig aren't (#604) so I manually tested the UI.
---
 CHANGELOG.md                                  |  1 +
 src/app/credExplorer/WeightConfig.js          | 44 +++++++++----------
 src/app/credExplorer/weights/WeightSlider.js  | 32 +++++++++-----
 .../credExplorer/weights/WeightSlider.test.js | 27 +++++++-----
 4 files changed, 57 insertions(+), 47 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 030d368..6c80696 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
 ## [Unreleased]
+- Improve weight sliders display (#736)
 - Separate bots from users in the UI (#720)
 - Add a feedback link to the prototype (#715)
 - Support combining multiple repositories into a single graph (#711)
diff --git a/src/app/credExplorer/WeightConfig.js b/src/app/credExplorer/WeightConfig.js
index 8b1ff60..08e19f6 100644
--- a/src/app/credExplorer/WeightConfig.js
+++ b/src/app/credExplorer/WeightConfig.js
@@ -16,24 +16,24 @@ type Props = {|
 
 type WeightedEdgeType = {|
   +type: EdgeType,
-  +logWeight: number,
+  +weight: number,
   +directionality: number,
 |};
 type EdgeWeights = WeightedEdgeType[];
 const defaultEdgeWeights = (): EdgeWeights => {
   const result = [];
   for (const type of defaultStaticAdapters().edgeTypes()) {
-    result.push({type, logWeight: 0, directionality: 0.5});
+    result.push({type, weight: 1.0, directionality: 0.5});
   }
   return result;
 };
 
 type NodeWeights = WeightedNodeType[];
-type WeightedNodeType = {|+type: NodeType, +logWeight: number|};
+type WeightedNodeType = {|+type: NodeType, +weight: number|};
 const defaultNodeWeights = (): NodeWeights => {
   const result = [];
   for (const type of defaultStaticAdapters().nodeTypes()) {
-    result.push({type, logWeight: type.defaultWeight});
+    result.push({type, weight: type.defaultWeight});
   }
   return result;
 };
@@ -97,16 +97,14 @@ export class WeightConfig extends React.Component<Props, State> {
 
   fire() {
     const {edgeWeights, nodeWeights} = this.state;
-    const edgePrefixes = edgeWeights.map(
-      ({type, logWeight, directionality}) => ({
-        prefix: type.prefix,
-        weight: 2 ** logWeight,
-        directionality,
-      })
-    );
-    const nodePrefixes = nodeWeights.map(({type, logWeight}) => ({
+    const edgePrefixes = edgeWeights.map(({type, weight, directionality}) => ({
       prefix: type.prefix,
-      weight: 2 ** logWeight,
+      weight,
+      directionality,
+    }));
+    const nodePrefixes = nodeWeights.map(({type, weight}) => ({
+      prefix: type.prefix,
+      weight,
     }));
     const edgeEvaluator = byNodeType(byEdgeType(edgePrefixes), nodePrefixes);
     this.props.onChange(edgeEvaluator);
@@ -122,18 +120,18 @@ class EdgeConfig extends React.Component<{
       this.props.edgeWeights,
       ({type}) => type.prefix
     );
-    return sortedWeights.map(({type, directionality, logWeight}) => {
+    return sortedWeights.map(({type, directionality, weight}) => {
       const onChange = (value) => {
         const edgeWeights = this.props.edgeWeights.filter(
           (x) => x.type.prefix !== type.prefix
         );
-        edgeWeights.push({type, logWeight: value, directionality});
+        edgeWeights.push({type, weight: value, directionality});
         this.props.onChange(edgeWeights);
       };
       return (
         <WeightSlider
           key={type.prefix}
-          weight={logWeight}
+          weight={weight}
           name={`${type.forwardName} / ${type.backwardName}`}
           onChange={onChange}
         />
@@ -146,12 +144,12 @@ class EdgeConfig extends React.Component<{
       this.props.edgeWeights,
       ({type}) => type.prefix
     );
-    return sortedWeights.map(({type, directionality, logWeight}) => {
+    return sortedWeights.map(({type, directionality, weight}) => {
       const onChange = (value: number) => {
         const edgeWeights = this.props.edgeWeights.filter(
           (x) => x.type.prefix !== type.prefix
         );
-        edgeWeights.push({type, directionality: value, logWeight});
+        edgeWeights.push({type, directionality: value, weight});
         this.props.onChange(edgeWeights);
       };
       return (
@@ -167,7 +165,7 @@ class EdgeConfig extends React.Component<{
   render() {
     return (
       <div>
-        <h2>Edge weights (in log space)</h2>
+        <h2>Edge weights</h2>
         {this.weightControls()}
         <h2>Edge directionality</h2>
         {this.directionControls()}
@@ -186,18 +184,18 @@ class NodeConfig extends React.Component<{
       ({type}) => type.prefix
     );
 
-    const controls = sortedWeights.map(({type, logWeight}) => {
+    const controls = sortedWeights.map(({type, weight}) => {
       const onChange = (value) => {
         const nodeWeights = this.props.nodeWeights.filter(
           (x) => x.type.prefix !== type.prefix
         );
-        nodeWeights.push({type, logWeight: value});
+        nodeWeights.push({type, weight: value});
         this.props.onChange(nodeWeights);
       };
       return (
         <WeightSlider
           key={type.prefix}
-          weight={logWeight}
+          weight={weight}
           name={type.name}
           onChange={onChange}
         />
@@ -205,7 +203,7 @@ class NodeConfig extends React.Component<{
     });
     return (
       <div>
-        <h2>Node weights (in log space)</h2>
+        <h2>Node weights</h2>
         {controls}
       </div>
     );
diff --git a/src/app/credExplorer/weights/WeightSlider.js b/src/app/credExplorer/weights/WeightSlider.js
index 2e96a2a..fdf9a46 100644
--- a/src/app/credExplorer/weights/WeightSlider.js
+++ b/src/app/credExplorer/weights/WeightSlider.js
@@ -9,28 +9,36 @@ export class WeightSlider extends React.Component<{|
 |}> {
   render() {
     return (
-      <label style={{display: "block"}}>
+      <label style={{display: "flex"}}>
+        <span style={{flexGrow: 1}}>{this.props.name}</span>
         <input
           type="range"
-          min={-10}
-          max={10}
-          step={0.1}
-          value={this.props.weight}
+          min={-5}
+          max={5}
+          step={1}
+          value={Math.log2(this.props.weight)}
           onChange={(e) => {
-            this.props.onChange(e.target.valueAsNumber);
+            const logValue = e.target.valueAsNumber;
+            this.props.onChange(2 ** logValue);
           }}
         />{" "}
-        <span>{formatWeight(this.props.weight)}</span>
-        <span>{this.props.name}</span>
+        <span
+          style={{minWidth: 45, display: "inline-block", textAlign: "right"}}
+        >
+          {formatWeight(this.props.weight)}
+        </span>
       </label>
     );
   }
 }
 
 export function formatWeight(n: number) {
-  let x = n.toFixed(1);
-  if (!x.startsWith("-")) {
-    x = "+" + x;
+  if (n <= 0 || !isFinite(n)) {
+    throw new Error(`Invalid weight: ${n}`);
+  }
+  if (n >= 1) {
+    return n.toFixed(0) + "×";
+  } else {
+    return `1/${(1 / n).toFixed(0)}×`;
   }
-  return x.replace("-", "\u2212");
 }
diff --git a/src/app/credExplorer/weights/WeightSlider.test.js b/src/app/credExplorer/weights/WeightSlider.test.js
index 44a24fb..6f65db1 100644
--- a/src/app/credExplorer/weights/WeightSlider.test.js
+++ b/src/app/credExplorer/weights/WeightSlider.test.js
@@ -16,16 +16,16 @@ describe("app/credExplorer/weights/WeightSlider", () => {
       );
       return {element, onChange};
     }
-    it("sets slider to the provided weight", () => {
+    it("sets slider to the log of provided weight", () => {
       const {element} = example();
-      expect(element.find("input").props().value).toBe(3);
+      expect(element.find("input").props().value).toBe(Math.log2(3));
     });
     it("prints the provided weight", () => {
       const {element} = example();
       expect(
         element
           .find("span")
-          .at(0)
+          .at(1)
           .text()
       ).toBe(formatWeight(3));
     });
@@ -34,28 +34,31 @@ describe("app/credExplorer/weights/WeightSlider", () => {
       expect(
         element
           .find("span")
-          .at(1)
+          .at(0)
           .text()
       ).toBe("foo");
     });
-    it("changes to the slider trigger the onChange", () => {
+    it("changes to the slider trigger the onChange with exponentiatied value", () => {
       const {element, onChange} = example();
       const input = element.find("input");
       input.simulate("change", {target: {valueAsNumber: 7}});
       expect(onChange).toHaveBeenCalledTimes(1);
-      expect(onChange).toHaveBeenCalledWith(7);
+      expect(onChange).toHaveBeenCalledWith(2 ** 7);
     });
   });
 
   describe("formatWeight", () => {
-    it("rounds to one decimal", () => {
-      expect(formatWeight(0.123)).toBe("+0.1");
+    it("shows numbers greater than 1 as a integer-rounded multiplier", () => {
+      expect(formatWeight(5.3)).toBe("5×");
     });
-    it("adds a + to 0", () => {
-      expect(formatWeight(0)).toBe("+0.0");
+    it("shows numbers less than 1 (but not 0) as integer-rounded fractions", () => {
+      expect(formatWeight(0.249)).toBe("1/4×");
     });
-    it("adds a minus symbol to negative numbers", () => {
-      expect(formatWeight(-3)).toBe("\u22123.0");
+    it("throws on bad values", () => {
+      const bads = [NaN, Infinity, -Infinity, -3, 0];
+      for (const bad of bads) {
+        expect(() => formatWeight(bad)).toThrowError("Invalid weight");
+      }
     });
   });
 });