Histogram is coming wrong + urgent

[nitin]

Hi
For the following observations
43.54
43.54
43.55
43.51
43.46
43.54
43.48
43.56
43.48
43.54
43.59
43.54
43.50
43.49
43.54
43.52
43.52
43.55
43.46
43.54
43.47
43.52
43.46
43.51
43.51
43.59
43.52

and
number of bins = 8

the output is coming wrong.
The interval for last bin was from 43.550 to 43.575.

where is the bin width calculated?
Pls help
Bye

[nitin]

Hi,
I forgot to add that the number of bins in the
chart generated was 7 instead of 8 that I had specified

Thanks

[Z.]

The bin width is calculated as

double binWidth = (maximum - minimum) / numberOfBins;

maximum and minimum is the max and min of the dataset.

it is calculated in the addSeries() method in the Histogramdataset class

Since I did not see anything is wrong with the logic there. I tried your data and found that it is the rounding issue.

[43.57375 43.589999999999996] is the calculated lower and upper boundary for the most left bin, so 43.59 , the largest point in the dataset never has a chance to get in any bins, that's why you only see 7 bins while you set it for 8.

I am working on the code and will post the fix very soon

Z.

[Z.]

here is the fix, go line 117 in the HistogramDataset class and find

for (int i = 0; i < bins.length; i++) {
HistogramBin bin = new HistogramBin(tmp, tmp + binWidth);
tmp = tmp + binWidth;
bins = bin;
}

replace it by

for (int i = 0; i < bins.length; i++)
{
HistogramBin bin = null;
// make sure bins[bins.length]'s upper boundary ends at maxium
// to avoid the rounding issue. the bins[0] lower boundary is
// guranteed strt from min
if ( i == bins.length -1)
bin = new HistogramBin(tmp, maximum);
else
bin = new HistogramBin(tmp, tmp + binWidth);
tmp = tmp + binWidth;
bins = bin;
}

[jelaiw]

Nice find guys (or girls). My data sets typically have over ten thousand points so this one slipped under my radar. All the more reason to write unit tests. I do have some unit tests written, but I haven't cleaned them up to work under David's unit testing framework yet.

Anyways, good job on finding this subtle bug. Are you going to submit the patch to sourceforge?

-jw

[david.gilbert]

Anyways, good job on finding this subtle bug. Are you going to submit the patch to sourceforge?

No need, I will add the change to CVS now for the upcoming 0.9.16 release.

[david.gilbert]

Done.

[juhi]

here is the fix, go line 117 in the HistogramDataset class and find

for (int i = 0; i < bins.length; i++) {
HistogramBin bin = new HistogramBin(tmp, tmp + binWidth);
tmp = tmp + binWidth;
bins = bin;
}

replace it by

for (int i = 0; i < bins.length; i++)
{
HistogramBin bin = null;
// make sure bins[bins.length]'s upper boundary ends at maxium
// to avoid the rounding issue. the bins[0] lower boundary is
// guranteed strt from min
if ( i == bins.length -1)
bin = new HistogramBin(tmp, maximum);
else
bin = new HistogramBin(tmp, tmp + binWidth);
tmp = tmp + binWidth;
bins = bin;
}

But in this case, we are assuming that the number of bins is given as a parameter and hence doe the rounding issue solution. what if i need to specify just the binwidth and let the number of bins be computed automatically?? in that case, wouldn't a little modification to the earlier code help? Please suggest and comment!