Skip to content
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.

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

The first and last bins of histograms are not drawn - FIXED #80

Closed
karlduderstadt opened this issue Jan 16, 2020 · 9 comments
Closed

The first and last bins of histograms are not drawn - FIXED #80

karlduderstadt opened this issue Jan 16, 2020 · 9 comments
Labels
bug:major Something isn't working

Comments

@karlduderstadt
Copy link

It seems that the first and last bins of histograms are not drawn. Is this the intended behavior of the render?

The following chart was generated with version 8.1.1 using the ErrorDataSetRenderer with two DefaultNumericAxis using PolyLineStyle LineStyle.HISTOGRAM and ErrorType ErrorStyle.NONE. The only dataset was a Histogram constructed using the double[] bins option to set the start and end position of all bins. It is just random sampling of a gaussian distribution.

Screenshot 2020-01-16 at 09 06 15

@wirew0rm
Copy link
Member

I can reproduce. This seems to be 2 (or even 3) different problems. on the left, the bar appears when panning a little bit to the left, on the right it shows when point reduction is turned off (renderer.setPointReduction(false)) but the last line segment is missing.
image

wirew0rm added a commit that referenced this issue Jan 16, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
@wirew0rm
Copy link
Member

I got a partial fix, but there is a deeper problem with rendering histograms with non-equidistant bins. The dataset interface for Histograms returns the center and therefore the renderer cannot reproduce the actual binning. This will probably need some deeper api changes to get it right.

Your sample looks pretty equidistant, so if your usecase is equidistant use the min/max/nBins constructor which will give you an equidistant Histogram which internally works a little different.

@wirew0rm wirew0rm added bug:minor workaround possible bug:major Something isn't working and removed bug:minor workaround possible labels Jan 16, 2020
wirew0rm added a commit that referenced this issue Jan 16, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
wirew0rm added a commit that referenced this issue Jan 16, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
@karlduderstadt
Copy link
Author

karlduderstadt commented Jan 17, 2020

Thanks so much for your fast response and very helpful feedback!

Yes, the bins are equidistant, so in principle the Histogram constructor that accepts min, max, and bin count would be ideal. Unfortunately, I ran into some trouble with that one which is why I switched to the constructor that just takes the list of bin boundaries.

When I plot the same data as I mentioned above with min = -2.0, max = 2.0, and bins = 10, I get the following histogram:
Screenshot 2020-01-16 at 21 55 09
As you can see all the bins are shifted toward the negative side by half a bin. I guess there is some shift introduced somewhere in the ErrorDataSetRenderer, unfortunately I haven't had time to look through and troubleshoot that code. I am sure you can more easily spot the issue and maybe fixing this would be a fast temporary solution.

Regarding the data reducer. For the current use case I turned it off and I don't think it is needed. I have noticed in general that if there are more than say 5-6 points in the dataset, the second to last point is always skipped. Even when zooming way in etc. If you like I can make a separate issue about that. Maybe I made a mistake in my implementation. I just thought I would mention it since you might have to look into the data reducer code based on your comments above.

wirew0rm added a commit that referenced this issue Jan 17, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
wirew0rm added a commit that referenced this issue Jan 17, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
RalphSteinhagen pushed a commit that referenced this issue Jan 17, 2020
Histogram returned wrong indices for non equidistant bins.
Partial fix for #80
@RalphSteinhagen
Copy link
Member

Hi @karlduderstadt, just as an update:
the first/last bin not being drawn seems to be solved now by PR #132.

The issue of 'half-a-bin-shift' seems to occur only for the non-equidistant binning option which is a bit special. The latter does not (yet) provide the necessary API info of where to draw the specific edges and unfortunately, this cannot be automatically determined. This is different for the equidistant version where this is solved implicitly.

Hope this helps. The changes should be merged/published within the coming weeks. Let us know if you have further questions or suggestions, otherwise, I'd like to close this issue.

@RalphSteinhagen
Copy link
Member

fixed by #132 and merged. closing issue

@RalphSteinhagen RalphSteinhagen changed the title The first and last bins of histograms are not drawn The first and last bins of histograms are not drawn - FIXED Mar 24, 2020
@karlduderstadt
Copy link
Author

karlduderstadt commented Mar 29, 2020

Hi @RalphSteinhagen,

Thanks very much for fixing this issue. Once you make a new release, I will test the fix and report back. Unfortunately, I haven't been able to configure my environment properly to compile and test out the unreleased version.

@wirew0rm
Copy link
Member

You could also use the snapshot build, we have recently added some documentation for that to our README.md: https://github.com/GSI-CS-CO/chart-fx#using-the-snapshot-repository

But it also won't be that long before the next release, so you can definitely also wait for that.
Thanks for keeping in touch.

@karlduderstadt
Copy link
Author

karlduderstadt commented May 9, 2020

So I finally had a chance to update to version 8.1.4 and test the histogram again. We use them everyday, but I just didn't have a chance to test the new version until now.

The first bin draws!! So that seems to be fixed. However, the last bin still don't draw for me. See image below:

Screenshot 2020-05-09 at 10 02 47

Am I missing something here. Is there some other way I should test this? Is there some way I can help. Perhaps if you point me to the relevant section of code, I can see if these is any solution I can implement on my side.

Again sorry I didn't test this earlier. Thanks very much for your efforts to fix this. You made such a wonderful charting library we rely completely on it a small library and gui we have written. Unfortunately, users have been complaining to me about the missing bins.

Thanks!!

@RalphSteinhagen
Copy link
Member

Hi @karlduderstadt

I gave it a quick try and appears to work:
image

Could you check this against the master and/or the JDK11 snapshot @wirew0rm mentioned above? Unfortunately, we haven't had the time to backport all recent changes to JDK8 yet but plan to do another x.1.5 release soonish.

N.B. This will probably also the last x.1.y. release before we'll increase the minor number since we want to introduce some API changes w.r.t. handling of 3D datasets etc. If you have suggestions/feature requests... there is an open issue #44.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:major Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants