Share default TimelineCredParameters (#1376)

At present, every place in the codebase that needs
TimelineCredParameters constructs them ad-hoc, meaning we don't have any
shared defaults across different consumers.

This commit adds a new type, `PartialTimelineCredParameters`, which
is basically `TimelineCredParameters` with every field marked optional.
Callers can then choose to override any fields where they want
non-default values. A new internal `partialParams` function promotes
these partial parameters to full parameters.

All the public interfaces for using params (namely,
`TimelineCred.compute` and `TimelineCred.reanalyze`) now accept optional
partial params. If the params are not specified, default values are
used; if partial params are provided, all the explicitly provided values
are used, and unspecified values are initialized to default values.

Test plan: A simple unit test was added to ensure that weights overrides
work as intended. `git grep "intervalDecay: "` reveals that there are no
other explicit parameter constructions in the codebase. All existing
unit tests pass.

src/analysis/timeline/params.js

@@ -5,6 +5,7 @@ import {
   type WeightsJSON,
   toJSON as weightsToJSON,
   fromJSON as weightsFromJSON,
+  defaultWeights,
 } from "../weights";
 
 /**
@@ -31,6 +32,20 @@ export type TimelineCredParameters = {|
   +weights: Weights,
 |};
 
+export const DEFAULT_ALPHA = 0.05;
+export const DEFAULT_INTERVAL_DECAY = 0.5;
+
+/**
+ * The PartialTimelineCredParameters are a version of TimelineCredParameters
+ * where every field has been marked optional, to make it convenient for API
+ * clients to override just the parameters they want to.
+ */
+export type PartialTimelineCredParameters = {|
+  +alpha?: number,
+  +intervalDecay?: number,
+  +weights?: Weights,
+|};
+
 export type TimelineCredParametersJSON = {|
   +alpha: number,
   +intervalDecay: number,
@@ -56,3 +71,32 @@ export function paramsFromJSON(
     weights: weightsFromJSON(p.weights),
   };
 }
+
+/**
+ * Exports the default TimelineCredParameters.
+ *
+ * End consumers of SourceCred will not need to depend on this; it's
+ * provided for implementation of SourceCred's APIs.
+ */
+export function defaultParams(): TimelineCredParameters {
+  return {
+    alpha: DEFAULT_ALPHA,
+    intervalDecay: DEFAULT_INTERVAL_DECAY,
+    weights: defaultWeights(),
+  };
+}
+
+/**
+ * Promote PartialTimelineCredParameters to TimelineCredParameters.
+ *
+ * This takes PartialTimelineCredParameters and mixes them with the
+ * default parameters to provide a full TimelineCredParameters.
+ *
+ * End consumers of SourceCred will not need to depend on this; it's
+ * provided for implementation of SourceCred's APIs.
+ */
+export function partialParams(
+  partial: PartialTimelineCredParameters
+): TimelineCredParameters {
+  return {...defaultParams(), ...partial};
+}

src/analysis/timeline/params.test.js

@@ -1,19 +1,67 @@
 // @flow
 
-import {paramsToJSON, paramsFromJSON} from "./params";
+import {
+  paramsToJSON,
+  paramsFromJSON,
+  defaultParams,
+  partialParams,
+  type TimelineCredParameters,
+  DEFAULT_ALPHA,
+  DEFAULT_INTERVAL_DECAY,
+} from "./params";
 import {defaultWeights} from "../weights";
 import {NodeAddress} from "../../core/graph";
 
 describe("analysis/timeline/params", () => {
-  it("JSON round trip", () => {
+  const customWeights = () => {
     const weights = defaultWeights();
     // Ensure it works with non-default weights
     weights.nodeManualWeights.set(NodeAddress.empty, 33);
-    const p = {alpha: 0.5, intervalDecay: 0.5, weights};
+    return weights;
+  };
+  it("JSON round trip", () => {
+    const p: TimelineCredParameters = {
+      alpha: 0.1337,
+      intervalDecay: 0.31337,
+      weights: customWeights(),
+    };
     const j = paramsToJSON(p);
     const p_ = paramsFromJSON(j);
     const j_ = paramsToJSON(p_);
     expect(j).toEqual(j_);
     expect(p).toEqual(p_);
   });
+  it("defaultParams", () => {
+    const expected: TimelineCredParameters = {
+      alpha: DEFAULT_ALPHA,
+      intervalDecay: DEFAULT_INTERVAL_DECAY,
+      weights: defaultWeights(),
+    };
+    expect(defaultParams()).toEqual(expected);
+  });
+  describe("partialParams", () => {
+    it("uses default values if no overrides provided", () => {
+      const params = partialParams({});
+      expect(params).toEqual(defaultParams());
+    });
+    it("accepts an alpha override", () => {
+      const params = partialParams({alpha: 0.99});
+      expect(params.weights).toEqual(defaultWeights());
+      expect(params.alpha).toEqual(0.99);
+      expect(params.intervalDecay).toEqual(DEFAULT_INTERVAL_DECAY);
+    });
+    it("accepts weights override", () => {
+      const weights = customWeights();
+      const params = partialParams({weights});
+      expect(params.weights).toEqual(weights);
+      expect(params.alpha).toEqual(DEFAULT_ALPHA);
+      expect(params.intervalDecay).toEqual(DEFAULT_INTERVAL_DECAY);
+    });
+    it("accepts intervalDecay override", () => {
+      const params = partialParams({intervalDecay: 0.1});
+      expect(params.weights).toEqual(defaultWeights());
+      expect(params.alpha).toEqual(DEFAULT_ALPHA);
+      expect(params.intervalDecay).toEqual(0.1);
+    });
+  });
 });

src/analysis/timeline/timelineCred.js

@@ -21,10 +21,12 @@ import {
   paramsToJSON,
   paramsFromJSON,
   type TimelineCredParametersJSON,
+  type PartialTimelineCredParameters,
+  partialParams,
+  defaultParams,
 } from "./params";
 
 export type {Interval} from "./interval";
-export type {TimelineCredParameters} from "./params";
 
 /**
  * A Graph Node wrapped with cred information.
@@ -85,8 +87,14 @@ export class TimelineCred {
    *
    * This returns a new TimelineCred; it does not modify the existing one.
    */
-  async reanalyze(newParams: TimelineCredParameters): Promise<TimelineCred> {
+  async reanalyze(
-    return await TimelineCred.compute(this._graph, newParams, this._plugins);
+    newParams: PartialTimelineCredParameters
+  ): Promise<TimelineCred> {
+    return await TimelineCred.compute({
+      graph: this._graph,
+      params: newParams,
+      plugins: this._plugins,
+    });
   }
 
   /**
@@ -217,11 +225,13 @@ export class TimelineCred {
     return new TimelineCred(graph, intervalsJSON, cred, params, pluginsJSON);
   }
 
-  static async compute(
+  static async compute(opts: {|
     graph: Graph,
-    params: TimelineCredParameters,
+    params?: PartialTimelineCredParameters,
-    plugins: $ReadOnlyArray<PluginDeclaration>
+    plugins: $ReadOnlyArray<PluginDeclaration>,
-  ): Promise<TimelineCred> {
+  |}): Promise<TimelineCred> {
+    const {graph, params, plugins} = opts;
+    const fullParams = params == null ? defaultParams() : partialParams(params);
     const nodeOrder = Array.from(graph.nodes()).map((x) => x.address);
     const types = combineTypes(plugins);
     const userTypes = [].concat(...plugins.map((x) => x.userTypes));
@@ -229,9 +239,9 @@ export class TimelineCred {
     const distribution = await timelinePagerank(
       graph,
       types,
-      params.weights,
+      fullParams.weights,
-      params.intervalDecay,
+      fullParams.intervalDecay,
-      params.alpha
+      fullParams.alpha
     );
     const cred = distributionToCred(
       distribution,
@@ -249,7 +259,7 @@ export class TimelineCred {
       graph,
       intervals,
       addressToCred,
-      params,
+      fullParams,
       plugins
     );
     return preliminaryCred.reduceSize({

src/analysis/timeline/timelineCred.test.js

@@ -11,8 +11,8 @@ import {
   EdgeAddress,
 } from "../../core/graph";
 import {TimelineCred} from "./timelineCred";
+import {defaultParams} from "./params";
 import {type PluginDeclaration} from "../pluginDeclaration";
-import {defaultWeights} from "../weights";
 import {type NodeType} from "../types";
 
 describe("src/analysis/timeline/timelineCred", () => {
@@ -84,8 +84,9 @@ describe("src/analysis/timeline/timelineCred", () => {
       const scores = intervals.map((_) => i);
       addressToCred.set(address, scores);
     }
-    const params = {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()};
+    return new TimelineCred(graph, intervals, addressToCred, defaultParams(), [
-    return new TimelineCred(graph, intervals, addressToCred, params, [plugin]);
+      plugin,
+    ]);
   }
 
   it("JSON serialization works", () => {

src/api/load.js

@@ -6,10 +6,9 @@ import path from "path";
 import {TaskReporter} from "../util/taskReporter";
 import {Graph} from "../core/graph";
 import {loadGraph} from "../plugins/github/loadGraph";
-import {
+import {TimelineCred} from "../analysis/timeline/timelineCred";
-  type TimelineCredParameters,
+import {defaultParams, partialParams} from "../analysis/timeline/params";
-  TimelineCred,
+import {type PartialTimelineCredParameters} from "../analysis/timeline/params";
-} from "../analysis/timeline/timelineCred";
 
 import {type Project} from "../core/project";
 import {setupProjectDirectory} from "../core/project_io";
@@ -19,7 +18,7 @@ import * as NullUtil from "../util/null";
 
 export type LoadOptions = {|
   +project: Project,
-  +params: TimelineCredParameters,
+  +params: ?PartialTimelineCredParameters,
   +plugins: $ReadOnlyArray<PluginDeclaration>,
   +sourcecredDirectory: string,
   +githubToken: string | null,
@@ -45,6 +44,7 @@ export async function load(
   taskReporter: TaskReporter
 ): Promise<void> {
   const {project, params, plugins, sourcecredDirectory, githubToken} = options;
+  const fullParams = params == null ? defaultParams() : partialParams(params);
   const loadTask = `load-${options.project.id}`;
   taskReporter.start(loadTask);
   const cacheDirectory = path.join(sourcecredDirectory, "cache");
@@ -101,7 +101,7 @@ export async function load(
   await fs.writeFile(graphFile, JSON.stringify(graph.toJSON()));
 
   taskReporter.start("compute-cred");
-  const cred = await TimelineCred.compute(graph, params, plugins);
+  const cred = await TimelineCred.compute({graph, params: fullParams, plugins});
   const credJSON = cred.toJSON();
   const credFile = path.join(projectDirectory, "cred.json");
   await fs.writeFile(credFile, JSON.stringify(credJSON));

src/api/load.test.js

@@ -19,6 +19,10 @@ import {NodeAddress, Graph} from "../core/graph";
 import {node} from "../core/graphTestUtil";
 import {TestTaskReporter} from "../util/taskReporter";
 import {load, type LoadOptions} from "./load";
+import {
+  type PartialTimelineCredParameters,
+  partialParams,
+} from "../analysis/timeline/params";
 
 type JestMockFn = $Call<typeof jest.fn>;
 jest.mock("../plugins/github/loadGraph", () => ({
@@ -70,7 +74,7 @@ describe("api/load", () => {
   // Tweaks the weights so that we can ensure we aren't overriding with default weights
   weights.nodeManualWeights.set(NodeAddress.empty, 33);
   // Deep freeze will freeze the weights, too
-  const params = deepFreeze({alpha: 0.05, intervalDecay: 0.5, weights});
+  const params: PartialTimelineCredParameters = {weights};
   const plugins = deepFreeze([]);
   const example = () => {
     const sourcecredDirectory = tmp.dirSync().name;
@@ -139,14 +143,10 @@ describe("api/load", () => {
   it("calls TimelineCred.compute with the right graph and options", async () => {
     const {options, taskReporter} = example();
     await load(options, taskReporter);
-    expect(timelineCredCompute).toHaveBeenCalledWith(
+    const args = timelineCredCompute.mock.calls[0][0];
-      expect.anything(),
+    expect(args.graph.equals(combinedGraph())).toBe(true);
-      params,
+    expect(args.params).toEqual(partialParams(params));
-      plugins
+    expect(args.plugins).toEqual(plugins);
-    );
-    expect(timelineCredCompute.mock.calls[0][0].equals(combinedGraph())).toBe(
-      true
-    );
   });
 
   it("saves the resultant cred.json to disk", async () => {

src/cli/load.js

@@ -9,6 +9,7 @@ import {defaultWeights, fromJSON as weightsFromJSON} from "../analysis/weights";
 import {load} from "../api/load";
 import {specToProject} from "../plugins/github/specToProject";
 import fs from "fs-extra";
+import {partialParams} from "../analysis/timeline/params";
 import {DEFAULT_PLUGINS} from "./defaultPlugins";
 
 function usage(print: (string) => void): void {
@@ -105,7 +106,7 @@ const loadCommand: Command = async (args, std) => {
   const projects = await Promise.all(
     projectSpecs.map((s) => specToProject(s, githubToken))
   );
-  const params = {alpha: 0.05, intervalDecay: 0.5, weights};
+  const params = partialParams({weights});
   const plugins = DEFAULT_PLUGINS;
   const optionses = projects.map((project) => ({
     project,

src/cli/load.test.js

@@ -11,6 +11,7 @@ import type {LoadOptions} from "../api/load";
 import {defaultWeights, toJSON as weightsToJSON} from "../analysis/weights";
 import * as Common from "./common";
 import {DEFAULT_PLUGINS} from "./defaultPlugins";
+import {defaultParams, partialParams} from "../analysis/timeline/params";
 
 import {makeRepoId, stringToRepoId} from "../core/repoId";
 
@@ -78,7 +79,7 @@ describe("cli/load", () => {
           discourseServer: null,
         },
         plugins: DEFAULT_PLUGINS,
-        params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
+        params: defaultParams(),
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,
         discourseKey: fakeDiscourseKey,
@@ -102,7 +103,7 @@ describe("cli/load", () => {
           repoIds: [stringToRepoId(projectId)],
           discourseServer: null,
         },
-        params: {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()},
+        params: defaultParams(),
         plugins: DEFAULT_PLUGINS,
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,
@@ -140,7 +141,7 @@ describe("cli/load", () => {
           repoIds: [makeRepoId("foo", "bar")],
           discourseServer: null,
         },
-        params: {alpha: 0.05, intervalDecay: 0.5, weights},
+        params: partialParams({weights}),
         plugins: DEFAULT_PLUGINS,
         sourcecredDirectory: Common.sourcecredDirectory(),
         githubToken: fakeGithubToken,

src/explorer/TimelineCredViewInspectionTest.js

@@ -6,7 +6,7 @@ import type {Assets} from "../webutil/assets";
 import {TimelineCredView} from "./TimelineCredView";
 import {Graph, NodeAddress} from "../core/graph";
 import {type Interval, TimelineCred} from "../analysis/timeline/timelineCred";
-import {defaultWeights} from "../analysis/weights";
+import {defaultParams} from "../analysis/timeline/params";
 
 export default class TimelineCredViewInspectiontest extends React.Component<{|
   +assets: Assets,
@@ -46,7 +46,7 @@ export default class TimelineCredViewInspectiontest extends React.Component<{|
       const scores = intervals.map((_unuesd, i) => generator(i));
       addressToCred.set(address, scores);
     }
-    const params = {alpha: 0.05, intervalDecay: 0.5, weights: defaultWeights()};
+    const params = defaultParams();
     return new TimelineCred(graph, intervals, addressToCred, params, []);
   }
 

src/explorer/TimelineExplorer.js

@@ -4,10 +4,8 @@ import React from "react";
 import deepEqual from "lodash.isequal";
 import {type Weights, copy as weightsCopy} from "../analysis/weights";
 import {type NodeAddressT} from "../core/graph";
-import {
+import {TimelineCred} from "../analysis/timeline/timelineCred";
-  TimelineCred,
+import {type TimelineCredParameters} from "../analysis/timeline/params";
-  type TimelineCredParameters,
-} from "../analysis/timeline/timelineCred";
 import {TimelineCredView} from "./TimelineCredView";
 import Link from "../webutil/Link";
 import {WeightConfig} from "./weights/WeightConfig";