mapbox/supercluster

Reduce not called for each point in cluster

giovanni-bertoncelli opened this issue · 11 comments

Hi, this is more a question that a bug report. I've noticed that in my application supercluster calls the reduce function is not called for each of the points in the final cluster, but at least one point is skipped. Am I missing something or it shouldn't behave like that?

Basically, reduce works like this: reduce(reduce(point1, point2), point3), so for the first reduce, it gets two points, and then acummulates the result with each subsequent point. Hope this helps.

Ok, @mourner thank you.. so how can I choose what to do in the first reduce?

since I do not know when the reduce function is called for the first time, it is not better, like in a "classic" reduce function, to call the function for the first time with an initial value? For example null or an empty object?

I probably found an issue... it seems that some props in the clusterization process gets lost if are not redeclared every time the reduce function is called... is this the expected behaviour? @mourner

Here the reproduction repo: https://stackblitz.com/edit/js-p6ulcw?file=index.js

@giovanni-bertoncelli map function needs to return a new object (maybe we should make the docs clearer about it). You're modifying an existing one, which leads to all kinds of bugs.

I think the problem is not there... In the original implementation I've returned a cloned object and the behaviour was the same.

I've updated also the repro repo...

@giovanni-bertoncelli looks like Supercluster works correctly. The issue happens when two clusters get merged — both might have different sets of status counters, so they all need to be merged, you can't rely on just one status.

Ok I see, thank you...
I think, like in this issue #150, maybe these subtle requirements on the usage of these function, should be documented... It can require spending a lot of time trying to debug them

@giovanni-bertoncelli I updated the docs to make the requirements clearer, but let me know if there's more worth mentioning.

Thank you! I'd probably mention that each reduce iteration should return the same set of keys in order to get the merge process right... Something like that