multiple selections on parcoords axes #2415

monfera · 2018-02-27T22:39:59Z

The branch so far contains the SVG part but not yet the WebGL part. Upcoming commits will have

working out the API part (Alex); currently, constraintrange is diverted to a new meaning
-> note: current attributes.js specification for constraintrange is just a hint for the current structure but not what it'll end up being, e.g. it now assumes two intervals
if needed, changes to interactions, eg. now a new brushing will make a new section, but perhaps a shift+brush is preferred
WebGL filtering (Robert)
added mock for testing multiselect features (Robert)
updated test cases as needed (mostly dependent on the API choices)
updated mocks due to pixel level differences and API changes
breakup of some functions or nested ternary conditions
solution for the remaining two lint issues

Changes:

factored out brushing
replaced d3.svg.brush with our own implementation supporting multiple selections
eliminated non-brush related logic from axisbrush.js for eventual reusability
general code improvements in areas that were in attention

Due to the need for supporting very numerous intervals (up to 1000 per axis) and the possibility for up to 60 axes, the SVG solution has to be economical, otherwise thousands of DOM elements would be added. I found that SVG slowness has a nontrivial component that scales with concurrently existing DOM element count, roughly; other
experiments show the technique of using a single DOM element for showing a lot of glyphs. The current solution uses two layers of stroke-dasharray to represent filter coverage but it's fairly easy to switch to eg. path (would need points ie. longer d than the current stroke-dasharray).

There subselections are not fused during drag interaction but they do get fused on mouse release.

More description to come. Code is liberally commented and glad to add more.

- replaced d3.svg.brush with our own implementation supporting multiple selections - eliminated non-brush related logic from axisbrush.js for eventual reusability - general code improvements in areas that were in attention

monfera · 2018-02-27T23:02:22Z

src/traces/parcoords/attributes.js

@@ -80,8 +80,22 @@ module.exports = {
            valType: 'info_array',
            role: 'info',
            items: [
-                {valType: 'number', editType: 'calc'},
-                {valType: 'number', editType: 'calc'}
+                {


This just shows the current structure, there won't be a limit of two specified intervals.

(for my own notes, @monfera don't worry about this unless it's blocking you)

If we merge in master, particularly #2399, we can use 2D and/or unbounded free-length info_array.
I'm not sure the best way to handle it though - since the vast majority of cases will still use a single range, I'd like to keep constraintrange 1D rather than forcing people to wrap it into 2D. So the two options that occur to me are:

make it a length-2N array, ie [min1, max1, min2, max2, min3, max3...] - this would be fairly simple for us to work with, and seems easy to describe to users, not sure how intuitive it will be for them to use though.

make a separate 2D attribute constraintranges that we switch to when there are multiple ranges. That would be more plumbing on our side, and we'd have to decide which one wins in a conflict, but the structure may be more intuitive.

@etpinard thoughts?

Users should not have to make constraintrange 2D to set a single constraint.

I'll like to add a third option, make constraintrange 1D OR 2D. This would be a first for info arrays, but note that some data arrays already use this pattern (e.g. heatmap x and y).

Another aspect is that the functionality is initially discussed specifically for ordinal dimensions, where it'd be natural to simply list values, eg. [1, 5, 8, 12], and even to mix singular values and ranges, like [1, [5, 8], 12] meaning 1, 5 to 8 and 12. It would be possible to support this, but it wouldn't blend well with using an 1D array for a single range, because then an 1D array with two elements could be interpreted as either a single range, or as two discrete values.

monfera · 2018-02-27T23:05:43Z

src/traces/parcoords/axisbrush.js

+                var intData = getInterval(b, d.unitScaleInOrder, y);
+                var unitRange = intData.interval;
+                var pixelRange = unitRange.map(d.unitScaleInOrder);
+                var s = b.svgBrush2;


upcoming commit will remove the 2 post-fix here and there (we used to have both D3 and our own brush for manual regression testing but not anymore)

monfera · 2018-02-27T23:09:15Z

src/traces/parcoords/parcoords.js

@@ -218,13 +233,36 @@ function viewModel(model) {
            xScale: xScale,
            x: xScale(i),
            canvasX: xScale(i) * canvasPixelRatio,
-            unitScale: unitScale(height, c.verticalPadding),
+            // fixme remove the old unitScale


unitScale is still being used for its inverse mapping due to how y values are arriving from the underlying d3 interaction callback calls, but could in theory be unified, simplifying the current two-step map eg. var y = d.unitScaleInOrder(d.unitScale.invert(d3.mouse(this)[1]));

- added bitmask tests while retaining the original 1D 'bounding box' test - removed SPLOM work seed - due to lightening up some of the shaders, do less work on the JS side too -> baseline case (1 range): 1/3 times the work in the vertex shader -> new use (>1 ranges): still rather fast

temporarily, so they'll work until we sort out the constraintrange attribute

and clean up remaining brush/brush2 cruft

- more precise mapping of constraints to data - which enables decreased filterEpsilon - only make the constraint mask texture once per render - and update it on subsequent renders, so we're not leaking THAT - let its height be independent of canvasHeight, and set at 2K - speed up mask generation by only unsetting, not unsetting and resetting - always overshoot ordinal values for clarity - when picking ordinal values, map closer to what your already showed instead of often jumping to cover an extra value

- use new constraintrange info_array 1-2 dimension format - add multiselect attribute - some further cleanup of conversions - image test to cover most of multiselect logic & precision - interaction tests of ordinal and continuous multiselect - put constraint ranges back into existing mocks

alexcjohnson · 2018-03-15T03:51:03Z

test/image/mocks/gl2d_parcoords.json

@@ -62,6 +62,8 @@
      },
      {
        "label": "Min height width",
+        "range": [200000, -30000],
+        "constraintrange": [0, 100000],


This is covered by the gl2d_parcoords_constraints mock too, but I added a reversed-axis constraint to this during my tests.

alexcjohnson · 2018-03-15T03:59:59Z

test/image/mocks/gl2d_parcoords_constraints.json

+        "constraintrange": [[-10, -5], [-1, 1], [5, 10]],
+        "values": [
+          -9, -9, -7, -7,
+          -10.001, 10.001, -4.98, 4.98, -10, 10, -5, 5,


Nearly every sample on this plot is testing something... the values arrays are broken into lines based on which dimension is being tested. This dimension is testing that precision on the outer bounds of the constraint range is limited by floating point (OK, I could have added another zero or two, 10.00001 or so...) and precision in the gaps between selection ranges is not as good, but this test (4.98) shows at least one thousandth of the range. The number of vertical pixels is 2048, and as expected if I move to 4.99 one of these values gets incorrectly marked valid. Notice that for this test to pass, all the lines going to -0.7 in the preceding and following dimensions should be grey, and all the lines going to 0.7 should be colored.

I didn't notice any performance issue with making the filter mask a fixed 2048 height, since it only has a width of 8. In principle we should be able to double that if more precision is required.

alexcjohnson · 2018-03-15T04:26:40Z

@etpinard I need to clean up some syntax and other test issues, but calling this reviewable as tests pass locally. Have a look!

…ontexts turns out my test case had unrealistic unique sorted data...

hopefully to avoid hangup due to webgl context destroy/create overload

alexcjohnson · 2018-03-15T19:55:41Z

test/jasmine/tests/parcoords_test.js

@@ -8,7 +8,7 @@ var attributes = require('@src/traces/parcoords/attributes');
 var createGraphDiv = require('../assets/create_graph_div');
 var delay = require('../assets/delay');
 var destroyGraphDiv = require('../assets/destroy_graph_div');
-var fail = require('../assets/fail_test');


FYI fail is a jasmine global, we were overloading it here, so among other things if you don't bring in assets/fail_test but you thought you had, linters won't catch it. We should probably change that in the rest of our test suites too...

alexcjohnson · 2018-03-16T01:20:39Z

I can’t figure out what’s going on with these tests. Seems like since I @noCi’d my new tests and it’s still hanging, perhaps the increased resource needs from multiselect are just reducing the threshold we can run before hanging. I guess I can look for the memory leak and see if fixing that allows more tests to run, or we can break the tests up further - shift more tests to @flaky, or even make a totally new tag...

alexcjohnson · 2018-03-16T04:20:59Z

I've reproduced the memory issue - it clearly behaves like a real leak, building up to multiple GB after 1000 or so lines are added in sequence, not dropping after you stop adding points and wait (so it's not just slow garbage collection) until you purge the plot with eg Plotly.newPlot.

Generated using this code - note CPU usage never went above ~20% during this test, but I couldn't leave the console open or the console process would eat 100% CPU and grind to a halt, though the plot would keep going just fine:

var trace = {type: 'parcoords', dimensions: []};
for(var i = 0; i < 5; i++) {
  trace.dimensions.push({values: [Math.random()], label: i, range: [0, 1]});
}
Plotly.newPlot(gd,[trace]);
var add = setInterval(function() {
  trace.dimensions.forEach(function(d) { d.values.push(Math.random()); });
  gd.layout.title = String(trace.dimensions[0].values.length);
  Plotly.redraw(gd);
}, 500);

Will look into this more tomorrow - perhaps looking back at older plotly.js versions to see where the leak was introduced (which is why I used redraw instead of react).

naive solution, cleaner one coming next, just want to see if this lets our CI tests run

and try reinstating my new tests from noCI-land

alexcjohnson · 2018-03-16T22:17:38Z

memory leak fixed -> 2911ae6 and cleaner create/update pattern in fa1a436
click on/near an ordinal value to toggle it (perhaps cursor: pointer to indicate this?) -> a7bd686

This reverts commit 03d8779.

alexcjohnson · 2018-03-19T01:08:47Z

@etpinard check it out, I added click-to-select for ordinal ranges. Should be ready to go.

etpinard · 2018-03-19T15:04:13Z

All right. Well, this is great! We should probably rename this PR Parcoords 2.0.

That click-to-select functionality is pretty sweet:

I won't pretend to understand more than 10% of the parcoods code (I would appreciate a walkthrough at some point 😉 ), but after some thorough testing, this thing deserves a 💃

- factored out brushing

9208cc3

- replaced d3.svg.brush with our own implementation supporting multiple selections - eliminated non-brush related logic from axisbrush.js for eventual reusability - general code improvements in areas that were in attention

monfera added feature something new status: in progress labels Feb 27, 2018

monfera commented Feb 27, 2018

View reviewed changes

monfera and others added 8 commits March 6, 2018 00:39

remove constraintrange from parcoords mocks

7f507c3

temporarily, so they'll work until we sort out the constraintrange attribute

tweak the parcoords constraint grabber ranges

d25d261

and clean up remaining brush/brush2 cruft

dimension.multiselect boolean attr

1030542

Merge branch 'master' into parcoords-multiselect-squashed

c046397

info_array dimensions='1-2'

a4f4a5f

alexcjohnson reviewed Mar 15, 2018

View reviewed changes

🔪 fdescribe

01ba11e

alexcjohnson changed the title ~~[WIP] multiple selections on parcoords axes~~ multiple selections on parcoords axes Mar 15, 2018

alexcjohnson added status: reviewable and removed status: in progress labels Mar 15, 2018

alexcjohnson added this to the v1.36.0 milestone Mar 15, 2018

alexcjohnson added 4 commits March 15, 2018 10:12

lint axisbrush

68e9e48

fail -> failTest - fail is a jasmine global

cb00901

fix one more filter bug, refactor parcoords test to hopefully reuse c…

f2690ac

…ontexts turns out my test case had unrealistic unique sorted data...

refactor more of parcoords test to use Plotly.react

ec44922

hopefully to avoid hangup due to webgl context destroy/create overload

alexcjohnson reviewed Mar 15, 2018

View reviewed changes

alexcjohnson added 3 commits March 15, 2018 17:03

parentElement -> parentNode

352a884

shorten parcoords snap tweening during tests

57085e1

@flaky on ordinal constraint snap test

fff668a

I give up... @noci my new tests

6fe2d79

alexcjohnson added 3 commits March 16, 2018 15:08

🔪 parcoords memory leak

2911ae6

naive solution, cleaner one coming next, just want to see if this lets our CI tests run

parcoords create/update pattern

fa1a436

parcoords test cleanup

0e75c5a

and try reinstating my new tests from noCI-land

alexcjohnson added 5 commits March 17, 2018 16:32

click to select an ordinal value

a7bd686

cleanup, and remove some obsolete code

6432b89

tweaked parcoords mocks

03d8779

Revert "tweaked parcoords mocks"

f147644

This reverts commit 03d8779.

fix bug introduced in cleanup

c3cc927

alexcjohnson merged commit f9a15be into master Mar 21, 2018

alexcjohnson deleted the parcoords-multiselect-squashed branch March 21, 2018 16:28

etpinard mentioned this pull request Mar 27, 2018

Point cluster dev #2499

Merged

etpinard mentioned this pull request Apr 13, 2018

Parcoords multiselection #2331

Closed

alexcjohnson mentioned this pull request Oct 18, 2018

Enable restyle to redraw parcoords #3101 #3125

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multiple selections on parcoords axes #2415

multiple selections on parcoords axes #2415

monfera commented Feb 27, 2018 •

edited

Loading

monfera Feb 27, 2018

alexcjohnson Mar 1, 2018

etpinard Mar 1, 2018 •

edited

Loading

monfera Mar 1, 2018

monfera Feb 27, 2018 •

edited

Loading

monfera Feb 27, 2018

alexcjohnson Mar 15, 2018

alexcjohnson Mar 15, 2018

alexcjohnson Mar 15, 2018

alexcjohnson commented Mar 15, 2018

alexcjohnson Mar 15, 2018

alexcjohnson commented Mar 16, 2018

alexcjohnson commented Mar 16, 2018

alexcjohnson commented Mar 16, 2018 •

edited

Loading

alexcjohnson commented Mar 19, 2018

etpinard commented Mar 19, 2018

multiple selections on parcoords axes #2415

multiple selections on parcoords axes #2415

Conversation

monfera commented Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard Mar 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monfera Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcjohnson commented Mar 15, 2018

Choose a reason for hiding this comment

alexcjohnson commented Mar 16, 2018

alexcjohnson commented Mar 16, 2018

alexcjohnson commented Mar 16, 2018 • edited Loading

alexcjohnson commented Mar 19, 2018

etpinard commented Mar 19, 2018

monfera commented Feb 27, 2018 •

edited

Loading

etpinard Mar 1, 2018 •

edited

Loading

monfera Feb 27, 2018 •

edited

Loading

alexcjohnson commented Mar 16, 2018 •

edited

Loading