From c7e5a3b87d7a94d8a15cacba13bd24f0baa3371b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dandelion=20Man=C3=A9?= Date: Tue, 4 Sep 2018 15:37:00 -0700 Subject: [PATCH] Configure forward/backward edge weights separately (#749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces a new component, `EdgeTypeConfig`, which is responsible for configuring the weights for a given edge type. The config creates two `WeightSlider`s: one for the forward direction, and one for the backward direction. The `DirectionalitySlider` is no longer used, and is removed. This fixes #596. So as to avoid confusion, we now describe every edge with variables, as in 'α REFERENCES β', and clarify that the weight modifies how cred flows from β to α. This necessitated the creation of an `EdgeWeightSlider`, local to the `EdgeTypeConfig`, which sets up a `WeightSlider` with the necessary greek characters. The EdgeTypeConfig is tested, so this is continuing progress towards solving #604. Test plan: I manually verified that modifying edge weights has the expected effect on cred scores. Also, some new unit tests are included. --- CHANGELOG.md | 1 + src/app/credExplorer/WeightConfig.js | 114 +++++++----------- src/app/credExplorer/edgeWeights.js | 26 ++-- .../weights/DirectionalitySlider.js | 39 ------ .../weights/DirectionalitySlider.test.js | 89 -------------- .../credExplorer/weights/EdgeTypeConfig.js | 77 ++++++++++++ .../weights/EdgeTypeConfig.test.js | 87 +++++++++++++ src/app/credExplorer/weights/WeightSlider.js | 7 +- .../credExplorer/weights/WeightSlider.test.js | 2 +- .../__snapshots__/EdgeTypeConfig.test.js.snap | 31 +++++ 10 files changed, 253 insertions(+), 220 deletions(-) delete mode 100644 src/app/credExplorer/weights/DirectionalitySlider.js delete mode 100644 src/app/credExplorer/weights/DirectionalitySlider.test.js create mode 100644 src/app/credExplorer/weights/EdgeTypeConfig.js create mode 100644 src/app/credExplorer/weights/EdgeTypeConfig.test.js create mode 100644 src/app/credExplorer/weights/__snapshots__/EdgeTypeConfig.test.js.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 02d5abf..e5f12f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Configure edge forward/backward weights separately (#749) - Combine "load graph" and "run pagerank" into one button (#759) - Store GitHub data compressed at rest, reducing space usage by 6–8× (#750) - Improve weight sliders display (#736) diff --git a/src/app/credExplorer/WeightConfig.js b/src/app/credExplorer/WeightConfig.js index 87afab5..0672863 100644 --- a/src/app/credExplorer/WeightConfig.js +++ b/src/app/credExplorer/WeightConfig.js @@ -6,35 +6,24 @@ import sortBy from "lodash.sortby"; import {type EdgeEvaluator} from "../../core/attribution/pagerank"; import {byEdgeType, byNodeType} from "./edgeWeights"; import {defaultStaticAdapters} from "../adapters/defaultPlugins"; -import type {EdgeType} from "../adapters/pluginAdapter"; -import {WeightSlider} from "./weights/WeightSlider"; -import {DirectionalitySlider} from "./weights/DirectionalitySlider"; import { NodeTypeConfig, defaultWeightedNodeType, type WeightedNodeType, } from "./weights/NodeTypeConfig"; +import { + EdgeTypeConfig, + defaultWeightedEdgeType, + type WeightedEdgeType, +} from "./weights/EdgeTypeConfig"; +import {styledVariable} from "./weights/EdgeTypeConfig"; type Props = {| +onChange: (EdgeEvaluator) => void, |}; -type WeightedEdgeType = {| - +type: EdgeType, - +weight: number, - +directionality: number, -|}; -type EdgeWeights = WeightedEdgeType[]; -const defaultEdgeWeights = (): EdgeWeights => { - const result = []; - for (const type of defaultStaticAdapters().edgeTypes()) { - result.push({type, weight: 1.0, directionality: 0.5}); - } - return result; -}; - type State = { - edgeWeights: EdgeWeights, + edgeWeights: $ReadOnlyArray, nodeWeights: $ReadOnlyArray, expanded: boolean, }; @@ -43,7 +32,9 @@ export class WeightConfig extends React.Component { constructor(props: Props): void { super(props); this.state = { - edgeWeights: defaultEdgeWeights(), + edgeWeights: defaultStaticAdapters() + .edgeTypes() + .map(defaultWeightedEdgeType), nodeWeights: defaultStaticAdapters() .nodeTypes() .map(defaultWeightedNodeType), @@ -94,11 +85,13 @@ export class WeightConfig extends React.Component { fire() { const {edgeWeights, nodeWeights} = this.state; - const edgePrefixes = edgeWeights.map(({type, weight, directionality}) => ({ - prefix: type.prefix, - weight, - directionality, - })); + const edgePrefixes = edgeWeights.map( + ({type, forwardWeight, backwardWeight}) => ({ + prefix: type.prefix, + forwardWeight, + backwardWeight, + }) + ); const nodePrefixes = nodeWeights.map(({type, weight}) => ({ prefix: type.prefix, weight, @@ -109,63 +102,38 @@ export class WeightConfig extends React.Component { } class EdgeConfig extends React.Component<{ - edgeWeights: EdgeWeights, - onChange: (EdgeWeights) => void, + edgeWeights: $ReadOnlyArray, + onChange: ($ReadOnlyArray) => void, }> { - weightControls() { - const sortedWeights = sortBy( - this.props.edgeWeights, - ({type}) => type.prefix - ); - return sortedWeights.map(({type, directionality, weight}) => { - const onChange = (value) => { - const edgeWeights = this.props.edgeWeights.filter( - (x) => x.type.prefix !== type.prefix + _renderWeightControls() { + return sortBy(this.props.edgeWeights, ({type}) => type.prefix).map( + (weightedEdgeType) => { + const onChange = (value) => { + const edgeWeights = this.props.edgeWeights.filter( + (x) => x.type.prefix !== weightedEdgeType.type.prefix + ); + edgeWeights.push(value); + this.props.onChange(edgeWeights); + }; + return ( + ); - edgeWeights.push({type, weight: value, directionality}); - this.props.onChange(edgeWeights); - }; - return ( - - ); - }); + } + ); } - directionControls() { - const sortedWeights = sortBy( - this.props.edgeWeights, - ({type}) => type.prefix - ); - 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, weight}); - this.props.onChange(edgeWeights); - }; - return ( - - ); - }); - } render() { return (

Edge weights

- {this.weightControls()} -

Edge directionality

- {this.directionControls()} +

+ Flow cred from {styledVariable("β")} to {styledVariable("α")} when: +

+ {this._renderWeightControls()}
); } diff --git a/src/app/credExplorer/edgeWeights.js b/src/app/credExplorer/edgeWeights.js index f645b6c..bc63519 100644 --- a/src/app/credExplorer/edgeWeights.js +++ b/src/app/credExplorer/edgeWeights.js @@ -10,20 +10,16 @@ import type {EdgeEvaluator} from "../../core/attribution/pagerank"; export function byEdgeType( prefixes: $ReadOnlyArray<{| +prefix: EdgeAddressT, - +weight: number, - +directionality: number, + +forwardWeight: number, + +backwardWeight: number, |}> ): EdgeEvaluator { const trie = new EdgeTrie(); - for (const weightedPrefix of prefixes) { - trie.add(weightedPrefix.prefix, weightedPrefix); + for (const {prefix, forwardWeight, backwardWeight} of prefixes) { + trie.add(prefix, {toWeight: forwardWeight, froWeight: backwardWeight}); } return function evaluator(edge: Edge) { - const {weight, directionality} = trie.getLast(edge.address); - return { - toWeight: directionality * weight, - froWeight: (1 - directionality) * weight, - }; + return trie.getLast(edge.address); }; } @@ -35,17 +31,17 @@ export function byNodeType( |}> ): EdgeEvaluator { const trie = new NodeTrie(); - for (const weightedPrefix of prefixes) { - trie.add(weightedPrefix.prefix, weightedPrefix); + for (const {weight, prefix} of prefixes) { + trie.add(prefix, weight); } return function evaluator(edge: Edge) { - const srcDatum = trie.getLast(edge.src); - const dstDatum = trie.getLast(edge.dst); + const srcWeight = trie.getLast(edge.src); + const dstWeight = trie.getLast(edge.dst); const baseResult = base(edge); return { - toWeight: dstDatum.weight * baseResult.toWeight, - froWeight: srcDatum.weight * baseResult.froWeight, + toWeight: dstWeight * baseResult.toWeight, + froWeight: srcWeight * baseResult.froWeight, }; }; } diff --git a/src/app/credExplorer/weights/DirectionalitySlider.js b/src/app/credExplorer/weights/DirectionalitySlider.js deleted file mode 100644 index 4a45b5e..0000000 --- a/src/app/credExplorer/weights/DirectionalitySlider.js +++ /dev/null @@ -1,39 +0,0 @@ -// @flow - -import React from "react"; - -function assertValidDirectionality(x: number) { - if (x < 0 || x > 1) { - throw new Error( - `directionality out of bounds: ${x} must be between 0 and 1` - ); - } -} - -export class DirectionalitySlider extends React.Component<{| - +directionality: number, - +name: string, - +onChange: (number) => void, -|}> { - render() { - assertValidDirectionality(this.props.directionality); - return ( - - ); - } -} diff --git a/src/app/credExplorer/weights/DirectionalitySlider.test.js b/src/app/credExplorer/weights/DirectionalitySlider.test.js deleted file mode 100644 index 9b9d092..0000000 --- a/src/app/credExplorer/weights/DirectionalitySlider.test.js +++ /dev/null @@ -1,89 +0,0 @@ -// @flow - -import React from "react"; -import {shallow} from "enzyme"; - -import {DirectionalitySlider} from "./DirectionalitySlider"; - -require("../../testUtil").configureEnzyme(); - -describe("app/credExplorer/weights/DirectionalitySlider", () => { - describe("DirectionalitySlider", () => { - function example() { - const onChange = jest.fn(); - const element = shallow( - - ); - return {element, onChange}; - } - it("sets slider to the provided weight", () => { - const {element} = example(); - expect(element.find("input").props().value).toBe(0.5); - }); - it("slider min is 0", () => { - const {element} = example(); - expect(element.find("input").props().min).toBe(0); - }); - it("slider max is 0", () => { - const {element} = example(); - expect(element.find("input").props().max).toBe(1); - }); - it("prints the provided weight", () => { - const {element} = example(); - expect( - element - .find("span") - .at(0) - .text() - ).toBe("0.50"); - }); - it("displays the provided name", () => { - const {element} = example(); - expect( - element - .find("span") - .at(1) - .text() - ).toBe("foo"); - }); - it("changes to the slider trigger the onChange", () => { - const {element, onChange} = example(); - const input = element.find("input"); - input.simulate("change", {target: {valueAsNumber: 0.99}}); - expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith(0.99); - }); - it("errors if provided an out-of-bound directionality", () => { - function withDirectionality(d) { - return () => - shallow( - - ); - } - expect(withDirectionality(-0.2)).toThrowError( - "directionality out of bounds" - ); - expect(withDirectionality(2)).toThrowError( - "directionality out of bounds" - ); - }); - it("errors rather than providing an out-of-bound directionality", () => { - const {element} = example(); - const input = element.find("input"); - expect(() => { - input.simulate("change", {target: {valueAsNumber: -0.2}}); - }).toThrowError("directionality out of bounds"); - expect(() => { - input.simulate("change", {target: {valueAsNumber: 2.0}}); - }).toThrowError("directionality out of bounds"); - }); - }); -}); diff --git a/src/app/credExplorer/weights/EdgeTypeConfig.js b/src/app/credExplorer/weights/EdgeTypeConfig.js new file mode 100644 index 0000000..20df2e3 --- /dev/null +++ b/src/app/credExplorer/weights/EdgeTypeConfig.js @@ -0,0 +1,77 @@ +// @flow + +import React from "react"; +import {WeightSlider, type Props as WeightSliderProps} from "./WeightSlider"; +import {type EdgeType} from "../../adapters/pluginAdapter"; + +export type WeightedEdgeType = {| + +type: EdgeType, + +forwardWeight: number, + +backwardWeight: number, +|}; + +export function defaultWeightedEdgeType(type: EdgeType): WeightedEdgeType { + return { + type, + forwardWeight: 1, + backwardWeight: 1, + }; +} + +export class EdgeTypeConfig extends React.Component<{ + +weightedType: WeightedEdgeType, + +onChange: (WeightedEdgeType) => void, +}> { + render() { + return ( +
+ { + this.props.onChange({ + ...this.props.weightedType, + forwardWeight, + }); + }} + /> + { + this.props.onChange({ + ...this.props.weightedType, + backwardWeight, + }); + }} + /> +
+ ); + } +} + +export function styledVariable(letter: string) { + return ( + // marginRight accounts for italicization + + {letter} + + ); +} + +export class EdgeWeightSlider extends React.Component { + render() { + const modifiedName = ( + + {styledVariable("α")} {this.props.name} {styledVariable("β")} + + ); + return ( + + ); + } +} diff --git a/src/app/credExplorer/weights/EdgeTypeConfig.test.js b/src/app/credExplorer/weights/EdgeTypeConfig.test.js new file mode 100644 index 0000000..2b1ebd7 --- /dev/null +++ b/src/app/credExplorer/weights/EdgeTypeConfig.test.js @@ -0,0 +1,87 @@ +// @flow + +import React from "react"; +import {shallow} from "enzyme"; + +import {WeightSlider} from "./WeightSlider"; +import { + defaultWeightedEdgeType, + EdgeTypeConfig, + EdgeWeightSlider, +} from "./EdgeTypeConfig"; +import {assemblesEdgeType} from "../../adapters/demoAdapters"; + +require("../../testUtil").configureEnzyme(); + +describe("app/credExplorer/weights/EdgeTypeConfig", () => { + describe("defaultWeightedEdgeType", () => { + it("sets default weights to 1, 1", () => { + const wet = defaultWeightedEdgeType(assemblesEdgeType); + expect(wet.forwardWeight).toEqual(1); + expect(wet.backwardWeight).toEqual(1); + }); + }); + describe("EdgeTypeConfig", () => { + function example() { + const onChange = jest.fn(); + const wet = { + type: assemblesEdgeType, + forwardWeight: 1, + backwardWeight: 0.5, + }; + const element = shallow( + + ); + const forwardSlider = element.find(EdgeWeightSlider).at(0); + const backwardSlider = element.find(EdgeWeightSlider).at(1); + return {onChange, wet, forwardSlider, backwardSlider}; + } + 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); + }); + 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); + }); + it("forward weight slider onChange works", () => { + const {wet, forwardSlider, onChange} = example(); + forwardSlider.props().onChange(9); + const updated = {...wet, forwardWeight: 9}; + 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}; + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toEqual(updated); + }); + }); + describe("EdgeWeightSlider", () => { + function example() { + const onChange = jest.fn(); + const element = shallow( + + ); + const weightSlider = element.find(WeightSlider); + return {element, onChange, weightSlider}; + } + it("renders the name along with some Greek characters", () => { + const {weightSlider} = example(); + const name = weightSlider.props().name; + expect(name).toMatchSnapshot(); + }); + it("passes through the weight unchanged", () => { + const {weightSlider} = example(); + expect(weightSlider.props().weight).toBe(3); + }); + it("onChange is wired properly", () => { + const {weightSlider, onChange} = example(); + expect(weightSlider.props().onChange).toBe(onChange); + }); + }); +}); diff --git a/src/app/credExplorer/weights/WeightSlider.js b/src/app/credExplorer/weights/WeightSlider.js index fdf9a46..289cb18 100644 --- a/src/app/credExplorer/weights/WeightSlider.js +++ b/src/app/credExplorer/weights/WeightSlider.js @@ -2,11 +2,12 @@ import React from "react"; -export class WeightSlider extends React.Component<{| +export type Props = {| +weight: number, - +name: string, + +name: React$Node, +onChange: (number) => void, -|}> { +|}; +export class WeightSlider extends React.Component { render() { return (