Problems sorting NaN and undefined values.

[jasondavies]

I've included a simple failing test case for NaN. However, it seems it can be more severe as my real dataset causes the call stack to overflow with "Uncaught RangeError: Maximum call stack size exceeded". I believe this only occurs when using the impressive dual-pivot quicksort i.e. arrays larger than the threshold of 32.

This may be of use. I'll have a closer look when I get time.

[jasondavies]

Hmm, this failing test case didn't actually reproduce the problem, because this quicksort implementation isn't stable.

I've pushed a new test that demonstrates the issue; namely taking ages to sort an array with 50% NaNs (does the same thing for undefined values). I think the stack limit in browsers must be much lower than in Node.js.

[jasondavies]

Okay, here's a fix! The issue is occurring because !(NaN <= NaN || NaN >= NaN) (likewise for undefined). Native sort moves these values to the end of the array, so for consistency it's simple enough to make a single pass and move such values to the end. Unfortunately it affects performance a bit, but perhaps it can be optimised by effectively performing this check on-the-fly elsewhere in the code.

I also wondered if caching coerced values would be useful e.g. to avoid calling valueOf twice for every comparison for dates, but perhaps smart JS VMs already optimise for this.

[eventualbuddha]

Out of curiosity, why do you want to sort NaN values?

[jasondavies]

My test data had a field that was undefined for a large number of cases, which has the same comparative qualities as NaN. It's simple enough to work around but I can imagine it happening a lot in practice as real-world data doesn't always have consistent fields.

[iros]

Major +1 to this.
Here is an example of how the filtering breaks if there are NaNs in the data to begin with:
http://jsfiddle.net/iros/wc8ba/3/

[john-guerra]

Thanks for this addition I really need it on my code, however I think there is still an error when you have half your numer of items or more NaNs in your data. I think this illustrates it (copying @iros jsfiddle)

http://jsfiddle.net/wc8ba/53/

[jasondavies]

@john-guerra Thank you for noticing this; I’ve fixed the issue and included a test case for bisect.right, which was the underlying cause. I’ve also merged with the latest master, so that jsfiddle no longer works due to the name change (tesseract→crossfilter).

[john-guerra]

@jasondavies It seems that the problem persists if the number of NaNs is big enough, check this case:

http://jsfiddle.net/wc8ba/94/

Am I missing something?

Thanks for looking at this, it would be great to get this fixed, I need it desperately for my code, so I offer my help in whatever I can

[jasondavies]

@john-guerra Fixed.

[john-guerra]

@jasondavies you sir have just won a big thank you note in my PhD dissertation! hehehe Thanks a lot!

[jasondavies]

Awesome. :)

[jeroen]

Great, thank you so much! Hope to see this make its way into the master soon!

[jasondavies]

Folded into #58.

[Mortimerp9]

this still seems to happen when using

crossfilter.dimension(function callback(pt) {
    //return undefined
});

It was a bug in my code that made this callback return undefined, but the stack overflow error is definitely not very helpful in finding out what's going on.

[TomNeyland]

@Mortimerp9

crossfilter.dimension(function callback(pt) {
});

Returning nothing causes the function to implicitly return undefined

[Mortimerp9]

Sorry if I wasn't clear, I have code in that callback that sometimes
returns undefined, this makes the quicksort call go in a infinite recursion
that blows the stack.

While the callback should avoid returning undefined, it would be good if
crossfilter guarded against it and threw a better error /warning as
debugging the stack overflow deep in quicksort is not the most
straightforward thing.
On Apr 21, 2015 4:14 PM, "Tom Neyland" notifications@github.com wrote:

@Mortimerp9 https://github.com/Mortimerp9

crossfilter.dimension(function callback(pt) {
});

Returning nothing causes the function to implicitly return 'undefined'

—
Reply to this email directly or view it on GitHub
#3 (comment).

[manojgn]

Hi guys, I am a great fan of crossfilter and have been using this in my app. I hit this exact same issue but I dont see the above fix in 1.3.12. Is it not merged or am I missing something ?

[jasondavies]

Some workarounds for incomparable values were attempted in versions v1.1.1-v1.1.3 but they were reverted as they caused too many issues: see the v1.2.0 release notes. Better to simply avoid using mixed types or incomparable values (undefined or NaN).