Skip to content

Add 'cumulative' histogram 'mode' for CDF #1189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 18, 2017
Merged

Add 'cumulative' histogram 'mode' for CDF #1189

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
62aeefb
[pof] add 'cumulative' histogram 'mode' for CDF
etpinard Nov 22, 2016
9c0dea0
Merge branch 'master' into cdf
alexcjohnson Jan 13, 2017
fd7526b
edge case in autoShiftNumericBins
alexcjohnson Jan 14, 2017
17c04c5
flesh out CDFs
alexcjohnson Jan 14, 2017
4bc28dd
further tweak of autoShiftNumericBins
alexcjohnson Jan 14, 2017
c12b7cf
fix bug in histogram min/max aggregation with normalization
alexcjohnson Jan 18, 2017
4d02af7
nest histogram cumulative attributes and test histnorm/histfunc more
alexcjohnson Jan 18, 2017
53b61aa
CDF test image
alexcjohnson Jan 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/plot_api/plot_api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,8 @@ function _restyle(gd, aobj, _traces) {
'tilt', 'tiltaxis', 'depth', 'direction', 'rotation', 'pull',
'line.showscale', 'line.cauto', 'line.autocolorscale', 'line.reversescale',
'marker.line.showscale', 'marker.line.cauto', 'marker.line.autocolorscale', 'marker.line.reversescale',
'xcalendar', 'ycalendar', 'cumulative', 'currentbin'
'xcalendar', 'ycalendar',
'cumulative', 'cumulative.enabled', 'cumulative.direction', 'cumulative.currentbin'
];

for(i = 0; i < traces.length; i++) {
Expand Down
90 changes: 49 additions & 41 deletions src/traces/histogram/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,59 +56,67 @@ module.exports = {
'If **, the span of each bar corresponds to the number of',
'occurrences (i.e. the number of data points lying inside the bins).',

'If *percent*, the span of each bar corresponds to the percentage',
'of occurrences with respect to the total number of sample points',
'(here, the sum of all bin area equals 100%).',
'If *percent* / *probability*, the span of each bar corresponds to',
'the percentage / fraction of occurrences with respect to the total',
'number of sample points',
'(here, the sum of all bin HEIGHTS equals 100% / 1).',

'If *density*, the span of each bar corresponds to the number of',
'occurrences in a bin divided by the size of the bin interval',
'(here, the sum of all bin area equals the',
'(here, the sum of all bin AREAS equals the',
'total number of sample points).',

'If *probability density*, the span of each bar corresponds to the',
'If *probability density*, the area of each bar corresponds to the',
'probability that an event will fall into the corresponding bin',
'(here, the sum of all bin area equals 1).'
'(here, the sum of all bin AREAS equals 1).'
].join(' ')
},

cumulative: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add one image mock. Maybe one that combines a currentbin: 'include' and currentbin: 'exclude' traces like in:

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test image in 53b61aa

One thing this showed is that we need a way to harmonize autobins across traces, and that it needs to know about cumulative. To make this example work I needed to manually extend the bin range for the smaller trace, otherwise its CDF ended too soon. Actually, CDFs never end, really... so perhaps the even better thing to do would be to look at the axis range and draw bins out to the edge. Anyway, fixing this will be a bigger project so I'll make an issue for it rather than try to address it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, fixing this will be a bigger project so I'll make an issue for it rather than try to address it here.

That's fine. Thanks for the info!

valType: 'boolean',
dflt: false,
role: 'info',
description: [
'If true, display the cumulative distribution by summing the',
'binned values. Use the `direction` and `centralbin` attributes',
'to tune the accumulation method.'
].join(' ')
},
enabled: {
valType: 'boolean',
dflt: false,
role: 'info',
description: [
'If true, display the cumulative distribution by summing the',
'binned values. Use the `direction` and `centralbin` attributes',
'to tune the accumulation method.',
'Note: in this mode, the *density* `histnorm` settings behave',
'the same as their equivalents without *density*:',
'** and *density* both rise to the number of data points, and',
'*probability* and *probability density* both rise to the',
'number of sample points.'
].join(' ')
},

direction: {
valType: 'enumerated',
values: ['increasing', 'decreasing'],
dflt: 'increasing',
role: 'info',
description: [
'Only applies if `cumulative=true.',
'If *increasing* (default) we sum all prior bins, so the result',
'increases from left to right. If *decreasing* we sum later bins',
'so the fresult decreases from left to right.'
].join(' ')
},
direction: {
valType: 'enumerated',
values: ['increasing', 'decreasing'],
dflt: 'increasing',
role: 'info',
description: [
'Only applies if cumulative is enabled.',
'If *increasing* (default) we sum all prior bins, so the result',
'increases from left to right. If *decreasing* we sum later bins',
'so the result decreases from left to right.'
].join(' ')
},

currentbin: {
valType: 'enumerated',
values: ['include', 'exclude', 'half'],
dflt: 'include',
role: 'info',
description: [
'Only applies if `cumulative=true.',
'Sets whether the current bin is included, excluded, or has half',
'of its value included in the current cumulative value.',
'*include* is the default for compatibility with various other',
'tools, however it introduces a half-bin bias to the results.',
'*exclude* makes the opposite half-bin bias, and *half* removes',
'it.'
].join(' ')
currentbin: {
valType: 'enumerated',
values: ['include', 'exclude', 'half'],
dflt: 'include',
role: 'info',
description: [
'Only applies if cumulative is enabled.',
'Sets whether the current bin is included, excluded, or has half',
'of its value included in the current cumulative value.',
'*include* is the default for compatibility with various other',
'tools, however it introduces a half-bin bias to the results.',
'*exclude* makes the opposite half-bin bias, and *half* removes',
'it.'
].join(' ')
}
},

autobinx: {
Expand Down
21 changes: 15 additions & 6 deletions src/traces/histogram/calc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ module.exports = function calc(gd, trace) {
trace.orientation === 'h' ? (trace.yaxis || 'y') : (trace.xaxis || 'x')),
maindata = trace.orientation === 'h' ? 'y' : 'x',
counterdata = {x: 'y', y: 'x'}[maindata],
calendar = trace[maindata + 'calendar'];
calendar = trace[maindata + 'calendar'],
cumulativeSpec = trace.cumulative;

cleanBins(trace, pa, maindata);

Expand All @@ -47,8 +48,8 @@ module.exports = function calc(gd, trace) {
binspec = Axes.autoBin(pos0, pa, trace['nbins' + maindata], false, calendar);

// adjust for CDF edge cases
if(trace.cumulative && (trace.currentbin !== 'include')) {
if(trace.direction === 'decreasing') {
if(cumulativeSpec.enabled && (cumulativeSpec.currentbin !== 'include')) {
if(cumulativeSpec.direction === 'decreasing') {
binspec.start = pa.c2r(pa.r2c(binspec.start) - binspec.size);
}
else {
Expand All @@ -74,8 +75,16 @@ module.exports = function calc(gd, trace) {
total = 0,
norm = trace.histnorm,
func = trace.histfunc,
densitynorm = norm.indexOf('density') !== -1,
extremefunc = func === 'max' || func === 'min',
densitynorm = norm.indexOf('density') !== -1;

if(cumulativeSpec.enabled && densitynorm) {
// we treat "cumulative" like it means "integral" if you use a density norm,
// which in the end means it's the same as without "density"
norm = norm.replace(/ ?density$/, '');
densitynorm = false;
}

var extremefunc = func === 'max' || func === 'min',
sizeinit = extremefunc ? null : 0,
binfunc = binFunctions.count,
normfunc = normFunctions[norm],
Expand Down Expand Up @@ -131,7 +140,7 @@ module.exports = function calc(gd, trace) {
if(normfunc) normfunc(size, total, inc);

// after all normalization etc, now we can accumulate if desired
if(trace.cumulative) cdf(size, trace.direction, trace.currentbin);
if(cumulativeSpec.enabled) cdf(size, cumulativeSpec.direction, cumulativeSpec.currentbin);


var serieslen = Math.min(pos.length, size.length),
Expand Down
6 changes: 3 additions & 3 deletions src/traces/histogram/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
var x = coerce('x'),
y = coerce('y');

var cumulative = coerce('cumulative');
var cumulative = coerce('cumulative.enabled');
if(cumulative) {
coerce('direction');
coerce('currentbin');
coerce('cumulative.direction');
coerce('cumulative.currentbin');
}

coerce('text');
Expand Down
78 changes: 56 additions & 22 deletions test/jasmine/tests/histogram_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,63 +247,97 @@ describe('Test histogram', function() {
});

describe('cumulative distribution functions', function() {
var base = {x: [1, 2, 3, 4, 2, 3, 4, 3, 4, 4]};
var base = {
x: [0, 5, 10, 15, 5, 10, 15, 10, 15, 15],
y: [2, 2, 2, 14, 6, 6, 6, 10, 10, 2]
};

it('makes the right base histogram', function() {
var baseOut = _calc(base);
expect(baseOut).toEqual([
{b: 0, p: 1, s: 1},
{b: 0, p: 2, s: 2},
{b: 0, p: 3, s: 3},
{b: 0, p: 4, s: 4},
{b: 0, p: 2, s: 1},
{b: 0, p: 7, s: 2},
{b: 0, p: 12, s: 3},
{b: 0, p: 17, s: 4},
]);
});

var CDFs = [
{p: [1, 2, 3, 4], s: [1, 3, 6, 10]},
{p: [2, 7, 12, 17], s: [1, 3, 6, 10]},
{
direction: 'decreasing',
p: [1, 2, 3, 4], s: [10, 9, 7, 4]
p: [2, 7, 12, 17], s: [10, 9, 7, 4]
},
{
currentbin: 'exclude',
p: [2, 3, 4, 5], s: [1, 3, 6, 10]
p: [7, 12, 17, 22], s: [1, 3, 6, 10]
},
{
direction: 'decreasing', currentbin: 'exclude',
p: [0, 1, 2, 3], s: [10, 9, 7, 4]
p: [-3, 2, 7, 12], s: [10, 9, 7, 4]
},
{
currentbin: 'half',
p: [1, 2, 3, 4, 5], s: [0.5, 2, 4.5, 8, 10]
p: [2, 7, 12, 17, 22], s: [0.5, 2, 4.5, 8, 10]
},
{
direction: 'decreasing', currentbin: 'half',
p: [0, 1, 2, 3, 4], s: [10, 9.5, 8, 5.5, 2]
p: [-3, 2, 7, 12, 17], s: [10, 9.5, 8, 5.5, 2]
},
{
direction: 'decreasing', currentbin: 'half', histnorm: 'percent',
p: [0, 1, 2, 3, 4], s: [100, 95, 80, 55, 20]
p: [-3, 2, 7, 12, 17], s: [100, 95, 80, 55, 20]
},
{
currentbin: 'exclude', histnorm: 'probability',
p: [2, 3, 4, 5], s: [0.1, 0.3, 0.6, 1]
p: [7, 12, 17, 22], s: [0.1, 0.3, 0.6, 1]
},
{
// behaves the same as without *density*
direction: 'decreasing', currentbin: 'half', histnorm: 'density',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

p: [-3, 2, 7, 12, 17], s: [10, 9.5, 8, 5.5, 2]
},
{
// behaves the same as without *density*, only *probability*
direction: 'decreasing', currentbin: 'half', histnorm: 'probability density',
p: [-3, 2, 7, 12, 17], s: [1, 0.95, 0.8, 0.55, 0.2]
},
{
currentbin: 'half', histfunc: 'sum',
p: [2, 7, 12, 17, 22], s: [1, 6, 19, 44, 60]
},
{
currentbin: 'half', histfunc: 'sum', histnorm: 'probability',
p: [2, 7, 12, 17, 22], s: [0.5 / 30, 0.1, 9.5 / 30, 22 / 30, 1]
},
{
direction: 'decreasing', currentbin: 'half', histfunc: 'max', histnorm: 'percent',
p: [-3, 2, 7, 12, 17], s: [100, 3100 / 32, 2700 / 32, 1900 / 32, 700 / 32]
},
{
direction: 'decreasing', currentbin: 'half', histfunc: 'min', histnorm: 'density',
p: [-3, 2, 7, 12, 17], s: [8, 7, 5, 3, 1]
},
{
currentbin: 'exclude', histfunc: 'avg', histnorm: 'probability density',
p: [7, 12, 17, 22], s: [0.1, 0.3, 0.6, 1]
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice to test cumulative: true with other histfunc and histnorm settings.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another good call by @etpinard 🌮 (and see also #1189 (comment))

What should we do with cumulative enabled and histnorm='density' or 'probability density'? As the code stands, CDFs using 'density' would rise to N/binSize (# samples / width of each bin) and 'probability density' would rise to 1/binSize. That seems useless and confusing, so I'd propose to interpret "cumulative" to mean an integral in these cases, ie 'density' would rise to N and 'probability density' would rise to 1, which then means in CDF mode these are equivalent to histnorm='' and 'probability' respectively.

I don't think there's anything special to do based on histfunc - some of these would also give strange results, but then the user is clearly asking for something strange.

Thoughts on any of this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's anything special to do based on histfunc - some of these would also give strange results, but then the user is clearly asking for something strange.

I can see this being used in time series CDFs. Think payments over time: bin by date and then cumulatively sum by payment amount

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd propose to interpret "cumulative" to mean an integral in these cases, ie 'density' would rise to N and 'probability density' would rise to 1

I agree 100% here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriddyp

I can see this being used in time series CDFs. Think payments over time: bin by date and then cumulatively sum by payment amount

Absolutely - and that will work just fine without modification (tests to come). I was just saying I don't think there's anything that needs altering based on histfunc, like what I'm planning to do for histnorm.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha! turns out we didn't have any tests with histfunc and histnorm together, and max/min were broken. Fixed in c12b7cf and tests for all of this (cumulative + histfunc + histnorm all in one go!) in 4d02af7

];

CDFs.forEach(function(CDF) {
var direction = CDF.direction,
currentbin = CDF.currentbin,
histnorm = CDF.histnorm,
p = CDF.p,
var p = CDF.p,
s = CDF.s;

it('handles direction=' + direction + ', currentbin=' + currentbin + ', histnorm=' + histnorm, function() {
it('handles direction=' + CDF.direction + ', currentbin=' + CDF.currentbin +
', histnorm=' + CDF.histnorm + ', histfunc=' + CDF.histfunc, function() {
var traceIn = Lib.extendFlat({}, base, {
cumulative: true,
direction: direction,
currentbin: currentbin,
histnorm: histnorm
cumulative: {
enabled: true,
direction: CDF.direction,
currentbin: CDF.currentbin
},
histnorm: CDF.histnorm,
histfunc: CDF.histfunc
});
var out = _calc(traceIn);

Expand Down