Weights: Refactor around a new EdgeWeight type (#1144)

This refactors the weights module (and downstream consumers) so that
rather than tracking forwardWeight and backwardsWeight as separate
fields, we have an `EdgeWeight` type with a forwards and backwards
property. I feel this makes the code a little cleaner and wanted it a
few times while doing other refactors in this module.

Note that the graphToMarkovChain module has a distinct `EdgeWeight` type
which serves a similar purpose but happens to have different field
names. I've added a TODO to rename those fields for consistency.

Test plan: Since this is slight data structure re-organization, no new
tests are required; that `yarn test` passes is sufficient.

src/analysis/fallbackDeclaration.js

@@ -20,8 +20,7 @@ export const fallbackNodeType: NodeType = Object.freeze({
 export const fallbackEdgeType: EdgeType = Object.freeze({
   forwardName: "points to",
   backwardName: "is pointed to by",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 1,
+  defaultWeight: {forwards: 1, backwards: 1},
   prefix: EdgeAddress.empty,
   description:
     "The fallback EdgeType for edges which don't have any other type",

src/analysis/types.js

@@ -1,6 +1,7 @@
 // @flow
 
 import {type NodeAddressT, type EdgeAddressT} from "../core/graph";
+import {type EdgeWeight} from "./weights";
 
 /**
  * This module defines `NodeType`s and `EdgeType`s, both of which are
@@ -86,15 +87,10 @@ export type EdgeType = {|
   // is "referenced by"
   +backwardName: string,
 
-  // The default weight for the forward direction of this edge.
+  // The default forwards and backwards weights for this edge.
   // We use `1` as a default value ("of normal importance").
   // The weights have linear importance, i.e. 2 is twice as important as 1.
-  +defaultForwardWeight: number,
-
-  // The default weight for the backward direction of this edge.
-  // We use `1` as a default value ("of normal importance").
-  // The weights have linear importance, i.e. 2 is twice as important as 1.
-  +defaultBackwardWeight: number,
+  +defaultWeight: EdgeWeight,
 
   // The address that will be used to test whether an edge is a member
   // of this EdgeType. A given edge `e` is a member of the type `t` if

src/analysis/weights.js

@@ -5,12 +5,26 @@ import {type NodeAddressT, type EdgeAddressT} from "../core/graph";
 import type {EdgeType, NodeType} from "./types";
 import type {PluginDeclaration} from "./pluginDeclaration";
 
-export type WeightedNodeType = {|+type: NodeType, +weight: number|};
+/**
+ * Represents the weight for a particular Node (or NodeType).
+ * Weight 1 is the default value and signifies normal importance.
+ * Weights are linear, so 2 is twice as important as 1.
+ */
+export type NodeWeight = number;
+
+/**
+ * Represents the forwards and backwards weights for a particular Edge (or
+ * EdgeType).
+ * Weight 1 is the default value and signifies normal importance.
+ * Weights are linear, so 2 is twice as important as 1.
+ */
+export type EdgeWeight = {|+forwards: number, +backwards: number|};
+
+export type WeightedNodeType = {|+type: NodeType, +weight: NodeWeight|};
 
 export type WeightedEdgeType = {|
   +type: EdgeType,
-  +forwardWeight: number,
-  +backwardWeight: number,
+  +weight: EdgeWeight,
 |};
 
 export type WeightedTypes = {|
@@ -31,8 +45,7 @@ export function defaultWeightedNodeType(type: NodeType): WeightedNodeType {
 export function defaultWeightedEdgeType(type: EdgeType): WeightedEdgeType {
   return {
     type,
-    forwardWeight: type.defaultForwardWeight,
-    backwardWeight: type.defaultBackwardWeight,
+    weight: type.defaultWeight,
   };
 }
 

src/analysis/weights.test.js

@@ -24,8 +24,8 @@ describe("analysis/weights", () => {
   describe("defaultWeightedEdgeType", () => {
     it("sets default weights as specified in the type", () => {
       const wet = defaultWeightedEdgeType(assemblesEdgeType);
-      expect(wet.forwardWeight).toEqual(wet.type.defaultForwardWeight);
-      expect(wet.backwardWeight).toEqual(wet.type.defaultBackwardWeight);
+      expect(wet.weight.forwards).toEqual(wet.type.defaultWeight.forwards);
+      expect(wet.weight.backwards).toEqual(wet.type.defaultWeight.backwards);
     });
   });
   describe("defaultWeightsForDeclaration", () => {

src/analysis/weightsToEdgeEvaluator.js

@@ -15,8 +15,8 @@ export function weightsToEdgeEvaluator(
     nodeTrie.add(type.prefix, weight);
   }
   const edgeTrie = new EdgeTrie();
-  for (const {type, forwardWeight, backwardWeight} of weights.edges.values()) {
-    edgeTrie.add(type.prefix, {forwardWeight, backwardWeight});
+  for (const {type, weight} of weights.edges.values()) {
+    edgeTrie.add(type.prefix, weight);
   }
 
   function nodeWeight(n: NodeAddressT): number {
@@ -28,10 +28,10 @@ export function weightsToEdgeEvaluator(
   return function evaluator(edge: Edge) {
     const srcWeight = nodeWeight(edge.src);
     const dstWeight = nodeWeight(edge.dst);
-    const {forwardWeight, backwardWeight} = edgeTrie.getLast(edge.address);
+    const {forwards, backwards} = edgeTrie.getLast(edge.address);
     return {
-      toWeight: dstWeight * forwardWeight,
-      froWeight: srcWeight * backwardWeight,
+      toWeight: dstWeight * forwards,
+      froWeight: srcWeight * backwards,
     };
   };
 }

src/analysis/weightsToEdgeEvaluator.test.js

@@ -43,13 +43,17 @@ describe("analysis/weightsToEdgeEvaluator", () => {
       const nodesMap = new Map(nodes.map((x) => [x.type.prefix, x]));
       const edges = [
         {
-          forwardWeight: NullUtil.orElse(assemblesForward, 1),
-          backwardWeight: NullUtil.orElse(assemblesBackward, 1),
+          weight: {
+            forwards: NullUtil.orElse(assemblesForward, 1),
+            backwards: NullUtil.orElse(assemblesBackward, 1),
+          },
           type: assemblesEdgeType,
         },
         {
-          forwardWeight: NullUtil.orElse(baseForward, 1),
-          backwardWeight: NullUtil.orElse(baseBackward, 1),
+          weight: {
+            forwards: NullUtil.orElse(baseForward, 1),
+            backwards: NullUtil.orElse(baseBackward, 1),
+          },
           type: fallbackEdgeType,
         },
       ];

src/cli/pagerank.test.js

@@ -206,8 +206,10 @@ describe("cli/pagerank", () => {
       const edgeType: EdgeType = {
         forwardName: "bars",
         backwardName: "barred by",
-        defaultForwardWeight: 5,
-        defaultBackwardWeight: 3,
+        defaultWeight: {
+          forwards: 5,
+          backwards: 3,
+        },
         prefix: EdgeAddress.fromParts(["hom"]),
         description: "an example edge type",
       };
@@ -268,14 +270,9 @@ describe("cli/pagerank", () => {
         expect(weightedNodeType.type).toEqual(nodeType);
       }
       for (const edgeType of declaration.edgeTypes) {
-        const weightedEdgeType = NullUtil.get(ws.edges.get(edgeType.prefix));
-        expect(weightedEdgeType.forwardWeight).toEqual(
-          edgeType.defaultForwardWeight
-        );
-        expect(weightedEdgeType.backwardWeight).toEqual(
-          edgeType.defaultBackwardWeight
-        );
-        expect(weightedEdgeType.type).toEqual(edgeType);
+        const {weight, type} = NullUtil.get(ws.edges.get(edgeType.prefix));
+        expect(weight).toEqual(edgeType.defaultWeight);
+        expect(type).toEqual(edgeType);
       }
     }
   });

src/core/attribution/graphToMarkovChain.js

@@ -97,6 +97,8 @@ export type OrderedSparseMarkovChain = {|
   +chain: SparseMarkovChain,
 |};
 
+// TODO(@decentralion): Rename these fields to `forwards` and `backwards` to
+// deduplicate with the EdgeWeight type defined by analysis/weights
 export type EdgeWeight = {|
   +toWeight: number, // weight from src to dst
   +froWeight: number, // weight from dst to src

src/explorer/pagerankTable/Aggregation.test.js

@@ -151,8 +151,7 @@ describe("explorer/pagerankTable/Aggregation", () => {
     const edgeType: EdgeType = {
       forwardName: "marsellus",
       backwardName: "wallace",
-      defaultForwardWeight: 1,
-      defaultBackwardWeight: 1,
+      defaultWeight: {forwards: 1, backwards: 1},
       prefix: EdgeAddress.fromParts(["look", "like"]),
       description: "Connects example nodes for testing purposes.",
     };

src/explorer/pagerankTable/aggregate.test.js

@@ -63,24 +63,21 @@ describe("explorer/pagerankTable/aggregate", () => {
       foo: {
         forwardName: "foos",
         backwardName: "foo'd by",
-        defaultForwardWeight: 1,
-        defaultBackwardWeight: 1,
+        defaultWeight: {forwards: 1, backwards: 1},
         prefix: EdgeAddress.fromParts(["foo"]),
         description: "Connects example foo edges.",
       },
       bar: {
         forwardName: "bars",
         backwardName: "bar'd by",
-        defaultForwardWeight: 1,
-        defaultBackwardWeight: 1,
+        defaultWeight: {forwards: 1, backwards: 1},
         prefix: EdgeAddress.fromParts(["bar"]),
         description: "Connects example bar edges.",
       },
       empty: {
         forwardName: "empty",
         backwardName: "emptied-by",
-        defaultForwardWeight: 1,
-        defaultBackwardWeight: 1,
+        defaultWeight: {forwards: 1, backwards: 1},
         prefix: EdgeAddress.empty,
         description: "Connects arbitrary edges.",
       },

src/explorer/weights/EdgeTypeConfig.js

@@ -10,27 +10,30 @@ export class EdgeTypeConfig extends React.Component<{
   +onChange: (WeightedEdgeType) => void,
 }> {
   render() {
+    const {weight, type} = this.props.weightedType;
+    const {forwards, backwards} = weight;
+    const {forwardName, backwardName, description} = type;
     return (
       <div>
         <EdgeWeightSlider
-          name={this.props.weightedType.type.backwardName}
-          weight={this.props.weightedType.forwardWeight}
-          description={this.props.weightedType.type.description}
-          onChange={(forwardWeight) => {
+          name={backwardName}
+          weight={forwards}
+          description={description}
+          onChange={(newForwards) => {
             this.props.onChange({
-              ...this.props.weightedType,
-              forwardWeight,
+              type,
+              weight: {forwards: newForwards, backwards},
             });
           }}
         />
         <EdgeWeightSlider
-          name={this.props.weightedType.type.forwardName}
-          weight={this.props.weightedType.backwardWeight}
-          description={this.props.weightedType.type.description}
-          onChange={(backwardWeight) => {
+          name={forwardName}
+          weight={backwards}
+          description={description}
+          onChange={(newBackwards) => {
             this.props.onChange({
-              ...this.props.weightedType,
-              backwardWeight,
+              type,
+              weight: {forwards, backwards: newBackwards},
             });
           }}
         />

src/explorer/weights/EdgeTypeConfig.test.js

@@ -15,8 +15,7 @@ describe("explorer/weights/EdgeTypeConfig", () => {
       const onChange = jest.fn();
       const wet = {
         type: assemblesEdgeType,
-        forwardWeight: 1,
-        backwardWeight: 0.5,
+        weight: {forwards: 1, backwards: 0.5},
       };
       const element = shallow(
         <EdgeTypeConfig onChange={onChange} weightedType={wet} />
@@ -28,12 +27,12 @@ describe("explorer/weights/EdgeTypeConfig", () => {
     it("sets up the forward weight slider", () => {
       const {wet, forwardSlider} = example();
       expect(forwardSlider.props().name).toBe(assemblesEdgeType.backwardName);
-      expect(forwardSlider.props().weight).toBe(wet.forwardWeight);
+      expect(forwardSlider.props().weight).toBe(wet.weight.forwards);
     });
     it("sets up the backward weight slider", () => {
       const {wet, backwardSlider} = example();
       expect(backwardSlider.props().name).toBe(assemblesEdgeType.forwardName);
-      expect(backwardSlider.props().weight).toBe(wet.backwardWeight);
+      expect(backwardSlider.props().weight).toBe(wet.weight.backwards);
     });
     it("has a description", () => {
       const {backwardSlider} = example();
@@ -44,14 +43,16 @@ describe("explorer/weights/EdgeTypeConfig", () => {
     it("forward weight slider onChange works", () => {
       const {wet, forwardSlider, onChange} = example();
       forwardSlider.props().onChange(9);
-      const updated = {...wet, forwardWeight: 9};
+      const weight = {...wet.weight, forwards: 9};
+      const updated = {...wet, weight};
       expect(onChange).toHaveBeenCalledTimes(1);
       expect(onChange.mock.calls[0][0]).toEqual(updated);
     });
     it("backward weight slider onChange works", () => {
       const {wet, backwardSlider, onChange} = example();
       backwardSlider.props().onChange(9);
-      const updated = {...wet, backwardWeight: 9};
+      const weight = {...wet.weight, backwards: 9};
+      const updated = {...wet, weight};
       expect(onChange).toHaveBeenCalledTimes(1);
       expect(onChange.mock.calls[0][0]).toEqual(updated);
     });

src/explorer/weights/PluginWeightConfig.test.js

@@ -130,8 +130,7 @@ describe("explorer/weights/PluginWeightConfig", () => {
       it("extra edge prefix in weighted types", () => {
         const badTypes = defaultWeightsForAdapter(new FactorioStaticAdapter());
         badTypes.edges.set(fallbackEdgeType.prefix, {
-          forwardWeight: 5,
-          backwardWeight: 3,
+          weight: {forwards: 5, backwards: 3},
           type: fallbackEdgeType,
         });
         checkError(badTypes);

src/plugins/demo/declaration.js

@@ -24,18 +24,16 @@ export const machineNodeType: NodeType = Object.freeze({
 
 export const assemblesEdgeType: EdgeType = Object.freeze({
   forwardName: "assembles",
-  defaultForwardWeight: 2,
   backwardName: "is assembled by",
-  defaultBackwardWeight: 2 ** -2,
+  defaultWeight: {forwards: 2, backwards: 2 ** -2},
   prefix: EdgeAddress.fromParts(["factorio", "assembles"]),
   description: "Connects assembly machines to products they assemble.",
 });
 
 export const transportsEdgeType: EdgeType = Object.freeze({
   forwardName: "transports",
-  defaultForwardWeight: 1,
   backwardName: "is transported by",
-  defaultBackwardWeight: 2 ** -1,
+  defaultWeight: {forwards: 2, backwards: 2 ** -1},
   prefix: EdgeAddress.fromParts(["factorio", "transports"]),
   description: "Connects transporter belts to objects they transport.",
 });

src/plugins/git/declaration.js

@@ -17,8 +17,7 @@ const hasParentEdgeType = Object.freeze({
   forwardName: "has parent",
   backwardName: "is parent of",
   prefix: E.Prefix.hasParent,
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 1,
+  defaultWeight: {forwards: 1, backwards: 1},
   description: "Connects a Git commit to its parent commit(s).",
 });
 

src/plugins/github/declaration.js

@@ -74,8 +74,7 @@ const nodeTypes = Object.freeze([
 const authorsEdgeType = Object.freeze({
   forwardName: "authors",
   backwardName: "is authored by",
-  defaultForwardWeight: 1 / 2,
-  defaultBackwardWeight: 1,
+  defaultWeight: {forwards: 1 / 2, backwards: 1},
   prefix: E.Prefix.authors,
   description: dedent`\
     Connects a GitHub account to a post that they authored.
@@ -87,8 +86,7 @@ const authorsEdgeType = Object.freeze({
 const hasParentEdgeType = Object.freeze({
   forwardName: "has parent",
   backwardName: "has child",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 1 / 4,
+  defaultWeight: {forwards: 1, backwards: 1 / 4},
   prefix: E.Prefix.hasParent,
   description: dedent`\
     Connects a GitHub entity to its child entities.
@@ -101,8 +99,7 @@ const hasParentEdgeType = Object.freeze({
 const mergedAsEdgeType = Object.freeze({
   forwardName: "merges",
   backwardName: "is merged by",
-  defaultForwardWeight: 1 / 2,
-  defaultBackwardWeight: 1,
+  defaultWeight: {forwards: 1 / 2, backwards: 1},
   prefix: E.Prefix.mergedAs,
   description: dedent`\
     Connects a GitHub pull request to the Git commit that it merges.
@@ -112,8 +109,7 @@ const mergedAsEdgeType = Object.freeze({
 const referencesEdgeType = Object.freeze({
   forwardName: "references",
   backwardName: "is referenced by",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 1, backwards: 0},
   prefix: E.Prefix.references,
   description: dedent`\
     Connects a GitHub post to an entity that it references.
@@ -127,8 +123,7 @@ const referencesEdgeType = Object.freeze({
 const mentionsAuthorEdgeType = Object.freeze({
   forwardName: "mentions author of",
   backwardName: "has author mentioned by",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 1, backwards: 0},
   prefix: E.Prefix.mentionsAuthor,
   description: dedent`\
     Connects a post that mentions a user to posts in the same thread that
@@ -144,8 +139,7 @@ const mentionsAuthorEdgeType = Object.freeze({
 const reactsHeartEdgeType = Object.freeze({
   forwardName: "reacted ❤️ to",
   backwardName: "got ❤️ from",
-  defaultForwardWeight: 2,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 2, backwards: 0},
   prefix: E.Prefix.reactsHeart,
   description: dedent`\
     Connects users to posts to which they gave a ❤️ reaction.
@@ -155,8 +149,7 @@ const reactsHeartEdgeType = Object.freeze({
 const reactsThumbsUpEdgeType = Object.freeze({
   forwardName: "reacted 👍 to",
   backwardName: "got 👍 from",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 1, backwards: 0},
   prefix: E.Prefix.reactsThumbsUp,
   description: dedent`\
     Connects users to posts to which they gave a 👍 reaction.
@@ -166,8 +159,7 @@ const reactsThumbsUpEdgeType = Object.freeze({
 const reactsHoorayEdgeType = Object.freeze({
   forwardName: "reacted 🎉 to",
   backwardName: "got 🎉 from",
-  defaultForwardWeight: 4,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 4, backwards: 0},
   prefix: E.Prefix.reactsHooray,
   description: dedent`\
     Connects users to posts to which they gave a 🎉 reaction.
@@ -177,8 +169,7 @@ const reactsHoorayEdgeType = Object.freeze({
 const reactsRocketEdgeType = Object.freeze({
   forwardName: "reacted 🚀 to",
   backwardName: "got 🚀 from",
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 1, backwards: 0},
   prefix: E.Prefix.reactsRocket,
   description: dedent`\
     Connects users to posts to which they gave a 🚀 reaction.

src/plugins/odyssey/declaration.js

@@ -64,8 +64,7 @@ const dependsOnEdgeType = Object.freeze({
   forwardName: "depends on",
   backwardName: "is depended on by",
   prefix: EdgeAddress.append(EDGE_PREFIX, "DEPENDS_ON"),
-  defaultForwardWeight: 1,
-  defaultBackwardWeight: 0,
+  defaultWeight: {forwards: 1, backwards: 0},
   description: "Generic edge for flowing credit in the Odyssey plugin",
 });