Merge pull request #42 from chanzuckerberg/bkmartinjr-perf2

performance - graph, scatterplot and brush select

src/actions/index.js

@@ -1,5 +1,6 @@
 // jshint esversion: 6
 import * as globals from "../globals";
+import store from "../reducers";
 import URI from "urijs";
 import _ from "lodash";
 
@@ -81,6 +82,30 @@ const initialize = () => {
   };
 };
 
+// This code defends against the case where /expression returns a cellname
+// never seen before (ie, not returned by /cells).   This should not happen
+// (see https://github.com/chanzuckerberg/cellxgene-rest-api/issues/34) but
+// occasionally does.
+//
+function cleanupExpressionResponse(data) {
+  const s = store.getState();
+  const metadata = s.controls.currentCellSelectionMap;
+  let errorOccured = false;
+  const newcells = _.filter(data.data.cells, cell => {
+    const found = metadata[cell.cellname];
+    errorOccured = errorOccured || !found;
+    return found;
+  });
+
+  if (errorOccured) {
+    console.error(
+      "Warning: /expression REST API returned unexpected cell names -- discarding surprises."
+    );
+    data.data.cells = newcells;
+  }
+  return data;
+}
+
 const requestGeneExpressionCounts = () => {
   return (dispatch, getState) => {
     dispatch({ type: "get expression started" });
@@ -91,6 +116,7 @@ const requestGeneExpressionCounts = () => {
       })
     })
       .then(res => res.json())
+      .then(data => cleanupExpressionResponse(data))
       .then(
         data => dispatch({ type: "get expression success", data }),
         error => dispatch({ type: "get expression error", error })
@@ -112,6 +138,7 @@ const requestSingleGeneExpressionCountsForColoringPOST = gene => {
       })
     })
       .then(res => res.json())
+      .then(data => cleanupExpressionResponse(data))
       .then(
         data =>
           dispatch({
@@ -142,6 +169,7 @@ const requestGeneExpressionCountsPOST = genes => {
       })
     })
       .then(res => res.json())
+      .then(data => cleanupExpressionResponse(data))
       .then(
         data => dispatch({ type: "get expression success", data }),
         error => dispatch({ type: "get expression error", error })

src/components/graph/graph.js

@@ -13,6 +13,7 @@ import fit from "canvas-fit";
 import _camera from "../../util/camera.js";
 import _regl from "regl";
 import _drawPoints from "./drawPointsRegl";
+import { scaleLinear } from "../../util/scaleLinear";
 
 import FaCrosshair from "react-icons/lib/fa/crosshairs";
 import FaZoom from "react-icons/lib/fa/search-plus";
@@ -41,7 +42,7 @@ import FaSave from "react-icons/lib/fa/download";
     colorAccessor: state.controls.colorAccessor,
     colorScale: state.controls.colorScale,
     continuousSelection: state.controls.continuousSelection,
-    graphMap: state.controls.graphMap,
+    graphVec: state.controls.graphVec,
     currentCellSelection: state.controls.currentCellSelection,
     graphBrushSelection: state.controls.graphBrushSelection,
     opacityForDeselectedCells: state.controls.opacityForDeselectedCells
@@ -103,72 +104,43 @@ class Graph extends React.Component {
       sizeBuffer
     });
   }
+
   componentWillReceiveProps(nextProps) {
-    /* maybe should do a check here to confirm ref exists and pass it? */
-    // if (
-    //   this.state.ctx &&
-    //   nextProps.vertices
-    //   // nextProps.expressions &&
-    //   // nextProps.expressionsCountsMap &&
-    // ) {
-    //   drawGraphUsingRenderQueue(
-    //     this.state.ctx,
-    //     nextProps.expressionsCountsMap,
-    //     nextProps.colorAccessor,
-    //     nextProps.ranges, /* assumption that this exists if vertices does both are on cells */
-    //     nextProps.metadata,
-    //     nextProps.currentCellSelection,
-    //     nextProps.graphBrushSelection,
-    //     nextProps.colorScale,
-    //     nextProps.graphMap,
-    //     nextProps.opacityForDeselectedCells,
-    //   )
-    // }
     if (this.state.regl && nextProps.vertices) {
-      const _currentCellSelectionMap = _.keyBy(
+      const vertices = nextProps.currentCellSelection;
-        nextProps.currentCellSelection,
+      const vertexCount = vertices.length;
-        "CellName"
+      const positions = new Float32Array(2 * vertexCount);
-      ); /* move me to the reducer */
+      const colors = new Float32Array(3 * vertexCount);
+      const sizes = new Float32Array(vertexCount);
 
-      const positions = [];
+      // d3.scaleLinear().domain([0,1]).range([-1,1])
-      positions.length = nextProps.currentCellSelection.length;
+      const glScaleX = scaleLinear([0, 1], [-1, 1]);
-      const colors = [];
+      // d3.scaleLinear().domain([0,1]).range([1,-1])
-      colors.length = nextProps.currentCellSelection.length;
+      const glScaleY = scaleLinear([0, 1], [1, -1]);
-      const sizes = [];
-      sizes.length = nextProps.currentCellSelection.length;
-
-      const glScaleX = d3
-        .scaleLinear()
-        .domain([0, 1])
-        .range([-1, 1]); /* padding */
-
-      const glScaleY = d3
-        .scaleLinear()
-        .domain([0, 1])
-        .range([1, -1]); /* padding */
 
       /*
         Construct Vectors
       */
-      _.each(nextProps.currentCellSelection, (cell, i) => {
+      const graphVec = nextProps.graphVec;
-        if (nextProps.graphMap[cell["CellName"]]) {
+      for (var i = 0; i < vertexCount; i++) {
-          /* fails silently, sometimes this is undefined, in which case the graph array should be shorter than the cell array, check in reducer */
+        const cell = vertices[i];
-          positions[i] = [
+        const cellIdx = cell.__cellIndex__;
-            glScaleX(nextProps.graphMap[cell["CellName"]][0]),
+        const x = glScaleX(graphVec[2 * cellIdx]);
-            glScaleY(nextProps.graphMap[cell["CellName"]][1])
+        const y = glScaleY(graphVec[2 * cellIdx + 1]);
-          ];
+        positions[2 * i] = x;
+        positions[2 * i + 1] = y;
 
-          colors[i] = cell.__colorRGB__;
+        colors.set(cell.__colorRGB__, 3 * i);
-          sizes[i] = cell["__selected__"]
-            ? 4
-            : 0.2; /* make this a function of the number of total cells, including regraph */
-        }
-      });
 
-      this.state.pointBuffer(positions);
+        sizes[i] = cell.__selected__
-      this.state.colorBuffer(colors);
+          ? 4
-      this.state.sizeBuffer(sizes);
+          : 0.2; /* make this a function of the number of total cells, including regraph */
-      this.count = positions.length;
+      }
+
+      this.state.pointBuffer({ data: positions, dimension: 2 });
+      this.state.colorBuffer({ data: colors, dimension: 3 });
+      this.state.sizeBuffer({ data: sizes, dimension: 1 });
+      this.count = vertexCount;
     }
   }
   handleBrushSelectAction() {

src/components/scatterplot/scatterplot.js

@@ -15,6 +15,7 @@ import fit from "canvas-fit";
 import _camera from "../../util/camera.js";
 import _regl from "regl";
 import _drawPoints from "./drawPointsRegl";
+import { scaleLinear } from "../../util/scaleLinear";
 
 import { margin, width, height, createDimensions } from "./util";
 
@@ -39,6 +40,7 @@ import { margin, width, height, createDimensions } from "./util";
     colorAccessor: state.controls.colorAccessor,
     colorScale: state.controls.colorScale,
     currentCellSelection: state.controls.currentCellSelection,
+    currentCellSelectionMap: state.controls.currentCellSelectionMap,
     scatterplotXXaccessor: state.controls.scatterplotXXaccessor,
     scatterplotYYaccessor: state.controls.scatterplotYYaccessor,
     opacityForDeselectedCells: state.controls.opacityForDeselectedCells,
@@ -107,12 +109,12 @@ class Scatterplot extends React.Component {
   }
   componentDidUpdate(prevProps) {
     if (
-      (this.state.xScale &&
+      this.state.xScale &&
-        this.state.yScale &&
+      this.state.yScale &&
-        (this.props.scatterplotXXaccessor &&
+      this.props.scatterplotXXaccessor &&
-          this.props.scatterplotYYaccessor) &&
+      this.props.scatterplotYYaccessor &&
-        this.props.scatterplotXXaccessor !== prevProps.scatterplotXXaccessor) || // was CLU now FTH1 etc
+      (this.props.scatterplotXXaccessor !== prevProps.scatterplotXXaccessor || // was CLU now FTH1 etc
-      this.props.scatterplotYYaccessor !== prevProps.scatterplotYYaccessor
+        this.props.scatterplotYYaccessor !== prevProps.scatterplotYYaccessor)
     ) {
       this.drawAxesSVG(this.state.xScale, this.state.yScale);
     }
@@ -130,67 +132,54 @@ class Scatterplot extends React.Component {
       this.state.xScale &&
       this.state.yScale
     ) {
-      const _currentCellSelectionMap = _.keyBy(
+      const currentCellSelectionMap = this.props.currentCellSelectionMap;
-        this.props.currentCellSelection,
-        "CellName"
-      ); /* move me to the reducer */
 
-      const positions = [];
+      const data = this.props.expression.data;
-      const colors = [];
+      const cells = data.cells;
-      const sizes = [];
+      const genes = data.genes;
+      const cellCount = cells.length;
+      const positions = new Float32Array(2 * cellCount);
+      const colors = new Float32Array(3 * cellCount);
+      const sizes = new Float32Array(cellCount);
 
-      const glScaleX = d3
+      // d3.scaleLinear().domain([0, width]).range([-0.95, 0.95])
-        .scaleLinear()
+      const glScaleX = scaleLinear([0, width], [-0.95, 0.95]);
-        .domain([0, width])
-        .range([-0.95, 0.95]); /* padding */
 
-      const glScaleY = d3
+      // d3.scaleLinear().domain([0, height]).range([-1, 1])
-        .scaleLinear()
+      const glScaleY = scaleLinear([0, height], [-1, 1]);
-        .domain([0, height])
+
-        .range([-1, 1]);
+      const geneXXaccessorIndex = genes.indexOf(
+        this.props.scatterplotXXaccessor
+      );
+      const geneYYaccessorIndex = genes.indexOf(
+        this.props.scatterplotYYaccessor
+      );
 
       /*
         Construct Vectors
       */
-      _.each(this.props.expression.data.cells, (cell, i) => {
+      for (var i = 0; i < cellCount; i++) {
-        /*
+        const cell = cells[i];
-          this if is necessary until we are no longer getting expression for all cells, but only for 'world'
+        const cellMetadata = currentCellSelectionMap[cell.cellname];
-          ...which will mean refetching when we regraph, or 'go back up to all cells'
-        */
-        if (_currentCellSelectionMap[cell.cellname]) {
-          /* fails silently, sometimes this is undefined, in which case the graph array should be shorter than the cell array, check in reducer */
-          positions.push([
-            glScaleX(
-              this.state.xScale(
-                cell.e[
-                  this.props.expression.data.genes.indexOf(
-                    this.props.scatterplotXXaccessor
-                  )
-                ]
-              )
-            ) /* scale each point first to the window as we calculate extents separately below, so no need to repeat */,
-            glScaleY(
-              this.state.yScale(
-                cell.e[
-                  this.props.expression.data.genes.indexOf(
-                    this.props.scatterplotYYaccessor
-                  )
-                ]
-              )
-            )
-          ]);
 
-          colors.push(_currentCellSelectionMap[cell.cellname]["__colorRGB__"]);
+        positions[2 * i] = glScaleX(
-          sizes.push(
+          this.state.xScale(cell.e[geneXXaccessorIndex])
-            _currentCellSelectionMap[cell.cellname]["__selected__"] ? 4 : 0.2
+        ); /* scale each point first to the window as we calculate extents separately below, so no need to repeat */
-          ); /* make this a function of the number of total cells, including regraph */
+        positions[2 * i + 1] = glScaleY(
-        }
+          this.state.yScale(cell.e[geneYYaccessorIndex])
-      });
+        );
 
-      this.state.pointBuffer(positions);
+        colors.set(cellMetadata.__colorRGB__, 3 * i);
-      this.state.colorBuffer(colors);
+
-      this.state.sizeBuffer(sizes);
+        sizes[i] = cellMetadata.__selected__
-      this.count = positions.length;
+          ? 4
+          : 0.2; /* make this a function of the number of total cells, including regraph */
+      }
+
+      this.state.pointBuffer({ data: positions, dimension: 2 });
+      this.state.colorBuffer({ data: colors, dimension: 3 });
+      this.state.sizeBuffer({ data: sizes, dimension: 1 });
+      this.count = cellCount;
     }
   }
   maybeSetupScalesAndDrawAxes(nextProps) {

src/globals.js

@@ -72,21 +72,25 @@ export const graphMargin = { top: 20, right: 10, bottom: 30, left: 40 };
 export const graphWidth = 960;
 export const graphHeight = 960;
 
-export const graphXScale = d3
+import { scaleLinear } from "./util/scaleLinear";
-  .scaleLinear()
+// d3.scaleLinear().domain([0,1]).range([0 + graphMargin.left, graphWidth - graphMargin.right])
-  .domain([
+export const graphXScale = scaleLinear(
-    0,
+  [0, 1],
-    1
+  [0 + graphMargin.left, graphWidth - graphMargin.right]
-  ]) /* while this is the default for d3, our data is normalized so better to be explicit */
+);
-  .range([0 + graphMargin.left, graphWidth - graphMargin.right]);
+graphXScale.invert = scaleLinear(
-
+  [0 + graphMargin.left, graphWidth - graphMargin.right],
-export const graphYScale = d3
+  [0, 1]
-  .scaleLinear()
+);
-  .domain([
+// d3.scaleLinear().domain([0,1]).range([graphHeight - graphMargin.bottom, 0 + graphMargin.top])
-    0,
+export const graphYScale = scaleLinear(
-    1
+  [0, 1],
-  ]) /* while this is the default for d3, our data is normalized so better to be explicit */
+  [graphHeight - graphMargin.bottom, 0 + graphMargin.top]
-  .range([graphHeight - graphMargin.bottom, 0 + graphMargin.top]);
+);
+graphYScale.invert = scaleLinear(
+  [graphHeight - graphMargin.bottom, 0 + graphMargin.top],
+  [0, 1]
+);
 
 export const ordinalColors = [
   "#0ac115",

src/middleware/updateCellColors.js

@@ -54,11 +54,12 @@ const updateCellSelectionMiddleware = store => {
       if (action.type === "color by categorical metadata") {
         colorScale = d3.scaleOrdinal().range(globals.ordinalColors);
 
-        _.each(currentSelectionWithUpdatedColors, (cell, i) => {
+        for (let i = 0; i < currentSelectionWithUpdatedColors.length; i++) {
+          const cell = currentSelectionWithUpdatedColors[i];
           let c = colorScale(cell[action.colorAccessor]);
-          currentSelectionWithUpdatedColors[i]["__color__"] = c;
+          cell.__color__ = c;
-          currentSelectionWithUpdatedColors[i]["__colorRGB__"] = parseRGB(c);
+          cell.__colorRGB__ = parseRGB(c);
-        });
+        }
       }
 
       if (action.type === "color by continuous metadata") {

src/middleware/updateCellSelectionMiddleware.js

@@ -37,7 +37,7 @@ const updateCellSelectionMiddleware = store => {
       if (
         !filterJustChanged ||
         !s.controls.allCellsOnClient
-        /* graphMap is set at the same time as allCells, so we assume it exists */
+        /* graph is set at the same time as allCells, so we assume it exists */
       ) {
         return next(
           action
@@ -46,12 +46,11 @@ const updateCellSelectionMiddleware = store => {
 
       /*
         - make a FRESH copy of all of the cells
-        - metadata has cellname, and that's all we ever need (is a key to graphMap)
+        - metadata has cellname and index, and that's all we ever need to reference cell info
       */
       let newSelection = s.controls.currentCellSelection.slice(0);
-      _.each(newSelection, cell => {
+      _.forEach(newSelection, cell => (cell.__selected__ = true));
-        cell["__selected__"] = true;
+
-      });
       /*
          in plain language...
 
@@ -74,26 +73,36 @@ const updateCellSelectionMiddleware = store => {
             ? action.brushCoords
             : s.controls.graphBrushSelection;
 
-        _.each(newSelection, (cell, i) => {
+        const northwestX = globals.graphXScale.invert(
-          if (!s.controls.graphMap[cell["CellName"]]) {
+          graphBrushSelection.northwestX
-            newSelection[i][
+        );
-              "__selected__"
+        const southeastX = globals.graphXScale.invert(
-            ] = false; /* make a toggle in future */
+          graphBrushSelection.southeastX
-            return;
+        );
-          }
+        const northwestY = globals.graphYScale.invert(
+          graphBrushSelection.northwestY
+        );
+        const southeastY = globals.graphYScale.invert(
+          graphBrushSelection.southeastY
+        );
 
-          const coords = s.controls.graphMap[cell["CellName"]]; // [0.08005009151334168, 0.6907652173913044]
+        const graphVec = s.controls.graphVec;
+        for (let i = 0; i < newSelection.length; i++) {
+          const cell = newSelection[i];
+          const cellId = cell.__cellIndex__;
+          const x = graphVec[2 * cellId];
+          const y = graphVec[2 * cellId + 1];
 
           const pointIsInsideBrushBounds =
-            globals.graphXScale(coords[0]) >= graphBrushSelection.northwestX &&
+            x >= northwestX &&
-            globals.graphXScale(coords[0]) <= graphBrushSelection.southeastX &&
+            x <= southeastX &&
-            globals.graphYScale(coords[1]) >= graphBrushSelection.northwestY &&
+            y <= northwestY &&
-            globals.graphYScale(coords[1]) <= graphBrushSelection.southeastY;
+            y >= southeastY;
 
           if (!pointIsInsideBrushBounds) {
-            newSelection[i]["__selected__"] = false;
+            cell.__selected__ = false;
           }
-        });
+        }
       }
       if (
         (action.type ===

src/reducers/controls.js

@@ -8,8 +8,9 @@ const Controls = (
     allGeneNames: null,
     allCellsOnClient: null /* this comes from cells endpoint, this is world */,
     currentCellSelection: null /* this comes from user actions, all draw components use this, it is created by middleware */,
-    graphMap: null,
+    graphVec: null,
     categoricalAsBooleansMap: null,
+    categoricalAsCellsMap: null,
     colorAccessor: null,
     colorScale: null,
     opacityForDeselectedCells: 0.2,
@@ -33,12 +34,22 @@ const Controls = (
         allGeneNames: action.data.data.genes
       });
     case "request cells success":
-      const graphMap = {};
+      // Store the graph in a linear array for fast access.  Index into
-      const currentCellSelection = action.data.data.metadata.slice(0);
+      // the array by "cell index", which is stored as metadata field
-      _.each(action.data.data.graph, g => {
+      // __cellIndex__.
-        graphMap[g[0]] = [g[1], g[2]];
+      //
+      // Code below relies on the REST API guarantee that the graph and
+      // metadata are returned as arrays with the same order and length.
+      //
+      const graphVec = new Float32Array(2 * action.data.data.graph.length);
+      _.each(action.data.data.graph, (g, i) => {
+        graphVec[2 * i] = g[1];
+        graphVec[2 * i + 1] = g[2];
       });
 
+      const currentCellSelection = action.data.data.metadata.slice(0);
+      const currentCellSelectionMap = _.keyBy(currentCellSelection, "CellName");
+
       /*
       construct a copy of the ranges object that only has categorical
       replace all counts with bool flags
@@ -66,11 +77,12 @@ const Controls = (
         }
       });
 
-      _.each(currentCellSelection, cell => {
+      _.each(currentCellSelection, (cell, idx) => {
-        cell["__selected__"] = true;
+        cell.__cellIndex__ = idx;
-        cell["__color__"] =
+        cell.__selected__ = true;
+        cell.__color__ =
           "rgba(0,0,0,1)"; /* initial color for all cells in all charts */
-        cell["__colorRGB__"] = parseRGB(cell["__color__"]);
+        cell.__colorRGB__ = parseRGB(cell.__color__);
 
         // Add each cell to its categorical metadata set.
         _.forEach(cell, (_value, key) => {
@@ -87,7 +99,8 @@ const Controls = (
       return Object.assign({}, state, {
         allCellsOnClient: action.data.data,
         currentCellSelection,
-        graphMap,
+        currentCellSelectionMap,
+        graphVec,
         categoricalAsBooleansMap,
         categoricalAsCellsMap,
         continuousUserDefinedRanges,

src/util/scaleLinear.js

@@ -0,0 +1,18 @@
+// jshint esversion: 6
+
+// Substitute for a d3 linear scale - less flexible, more performant.
+// Returns a function which will scale a value.
+//
+// Example will scale [0,1] to [-1,1]
+//     var myScale = scaleLinear([0, 1], [-1, 1]);
+//     myScale(0) === -1
+// this is is equivalent to d3.scaleLinear().domain([0,1]).range([-1,1])
+
+export const scaleLinear = (domain, range) => {
+  const offsetD = domain[0];
+  const scale = (range[1] - range[0]) / (domain[1] - domain[0]);
+  const offsetR = range[0];
+  return function(v) {
+    return (v - offsetD) * scale + offsetR;
+  };
+};