ScatterPlot Class Too few legends / colors exception

[Xyclade]

In the Scatterplot implementation, when you add labels colors or legends, the implementation uses the maximum value of the labels, rather than the actual amount of labels.

I wonder if this implementation is intended this way, since this implies that the labels array should always contain values starting at 0 and increment by 1 to n (where n is the amount of labels one wants to use)

I'd suggest making a map that maps the unique values which you gain in line 129:
int[] id = Math.unique(y);

against the Colors and legends, and use these maps to gain the correct color, rather than using the label value for its color such as done now here on line 171:
g.setColor(palette[y[i]]);

I stumbled upon this with the following code giving the 'Too few legends exception' :

val exampleData = Array(Array(1.0,120.0),Array(2,123.0))
val exampleLabels = Array(1,2)
val exampleLegends = Array('@','*')

val plot =  ScatterPlot.plot(exampleData,exampleLabels,exampleLegends,Array( Color.blue, Color.green))

to elaborate

val exampleData = Array(Array(100.0,120.0),Array(200,123.0))
val exampleLabels = Array(100,200)
val exampleLegends = Array('@','*')

val plot =  ScatterPlot.plot(exampleData,exampleLabels,exampleLegends,Array( Color.blue, Color.green))

would require 201 colors and legend symbols. (instead of 2, for the 2 labels)

[haifengl]

Sorry, this is a documentation problem. The labels should be in the range [0, k), where k is the number of classes. In your sample code, we should have

val exampleLabels = Array(0, 1)

We could change Array to Map. However, there is a performance issue for large number of points. Some people tried to plot millions data points. Map will make the UI unusable.

[Xyclade]

Oh maybe I explained it wrongly:

My suggestion was to keep the array for the labels the same, but to change the implementation in the constructor, such that next to the array you get a global lookup table which holds the unique values and their Colors / legends. I'll work it out, such that you can see exactly what I mean, because explaining it is rather difficult.

Ps. It should not affect the performance, as the extra computation is only n where n is the amount of labels. At least, in the way I see the adjustment.

[haifengl]

The performance is not in the constructor but in plot. For each data point, we may need change color or legend, now it is done by a Map lookup instead of array index. In terms of complexity, it is still O(n) for HashMap. But the constant factor of n is much bigger now. In your experiments, please try millions data points to make sure if it is still ok. Maybe I am wrong. Thanks!

[Xyclade]

I will test with a lot of datapoints. This is the fix I'm suggesting. Since the lookup in the hashmap should be O(1) it should not affect the plot method in performance.

In a second commit I merged the two for loops as I missed the second one in the first commit.
I'll let you know how the test goes.

[Xyclade]

For 10 million datapoints the time to plot was equal (+/- 1 second on 1.04 minutes plotting time) for both methods.

[haifengl]

Cool. BTW, we don't need to check if id is negative now. Can you rename valueLookupTable to classLookupTable? Thanks!

I had a commit updating the comment. Can you please update the comment again? The API may be confusing now. The user may not have clear idea of what is the color/legend of class 100.

[Xyclade]

I removed the negative check and renamed the table. However I cannot find the commit regarding the comment, could you point me out where I should update this comment? 👍

[haifengl]

Sorry, forgot to push to github. It is there now. Thanks!

[Xyclade]

Tnx for merging it 😄