-
Notifications
You must be signed in to change notification settings - Fork 532
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
Adds PlotHistogram and PlotHistogram2D #148
Conversation
@marcizhu and @ozlb , this is a WIP inspired by your recent posts. I'm opening this up for your feedback now, but I'll make a more complete post when I get things closed to finished. One thing to note is that 2D histograms are currently upside down since I'm just piping them through heatmaps, so I need to add a bool to reverse the direction. I'm trying to include common functionality found in MATLAB and matplotlib without going overboard (e.g. density normalization, custom ranges, cumulation, etc). @marcizhu, I was playing around with bin sizes, and couldn't hit the 1024x256 bins @ 60FPS you spoke of. Hoping you can take a look and post your code for comparison. // Plots a horizontal histogram. If cumulative is true, each bin contains its count plus the counts of all previous bins. If density is true, the PDF is visualized. If both are true, the CDF is visualized.
// If range is left unspecified, the min/max of values will be used as the range. Values outside of range are not binned and are ignored in the total count for density=true histograms.
void PlotHistogram(const char* label_id, const T* values, int count, int bins, bool cumulative=false, bool density=false, ImPlotRange range=ImPlotRange(), double bar_scale=1.0);
// Plots two dimensional, bivariate histogram as a heatmap. If density is true, the PDF is visualized. If range is left unspecified,
// the min/max of xs an ys will be used as the ranges. Values outside of range are not binned and are ignored in the total count for density=true histograms.
void PlotHistogram2D(const char* label_id, const T* xs, const T* ys, int count, int x_bins, int y_bins, bool density=false, ImPlotLimits range=ImPlotLimits()); |
@epezent I haven't looked too deep into the code, but it looks like you're regenerating the signal on every frame. My code just generated the signal once and only added a normal distribution noise on top of it, which is a lot faster (but consumes more memory). My guess is that this is what's causing the different performance, since I had the same issue when I tried generating the signal every frame. I guess But leaving performance aside, I think this is a great addition to this library! I only have one question: is it possible to leave blank the number of bins and let the library figure it out by itself? I usually work with R and their histogram function automatically calculates the bin sizes using Sturge's Law, which is quite simple and it only depends on the size of the input vector. I think this might be a great (and easy) addition to this PR 😄 Thoughts? |
I had slightly different approach, but probably your results are better. |
Only the theoretical curve. The gaussian distributions are static. I suppose this could be it, but I doubt it. I just need to run the my profiler and investigate.
I added this. Set bins equal to one of these for auto binning. I was going to add Freedman–Diaconis' choice as it seems common, but it requires a quicksort for determining the IQR. I didn't have time for that this morning. // Enums for different automatic histogram binning methods (k = bin count or w = bin width)
enum ImPlotBinMethod_ {
ImPlotBinMethod_Sqrt = -1, // k = sqrt(n)
ImPlotBinMethod_Sturges = -2, // k = 1 + log2(n)
ImPlotBinMethod_Rice = -3, // k = 2 * cbrt(n)
ImPlotBinMethod_Scott = -4, // w = 3.49 * sigma / cbrt(n)
}; |
@epezent Awesome! I think this is a great addition to the library. 10/10 👏 👏 Thanks for adding this! :D |
I don't know if can help, but for sure you need to take in consideration that
This is the reason why I used Welford's method, to compute the variance in a single pass; here below a sketch of function to give you the idea (I extrapolate because is doing also other stuff and using external vars, sorry about that...) void OnlineStat(TDataSample xs, int count, double& xyMin, double& xyMax, double& ySum, double& varM, double& varS, double& yAvg, double& yAvg, double &yVariance) {
if (count < 2) return xs.y;
double oldM = varM;
ySum += xs.y;
varM += (xs.y - varM) / count;
varS += (xs.y - varM)*(xs.y - oldM);
if (xyMin.y >= xs.y)//= to get first minimum in the list
xyMin = TDataSample(xs.x, xs.y);
if (xyMax.y < xs.y)
xyMax = TDataSample(xs.x, xs.y);
yAvg = ySum / cnt;
//Variance Welford's method
yVariance = (varS / (count - 1));
}
//PDF (probability density function)
static double PDF(double y, double avg, double variance) {
const double inv_sqrt_2pi = 0.3989422804014327;
double a = (y - avg) / variance;
return (inv_sqrt_2pi / variance * exp(-(double)(0.5) * a * a));
} Inspired by |
@epezent This is the code that I use to get the sample that I sent a few days ago to #94: void renderHistogram(float* sinewave, float* values, const std::normal_distribution<float>& distribution)
{
ImGui::Begin("Test histogram", NULL, ImGuiWindowFlags_NoCollapse | ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoResize);
for(int i = 0; i < 1024; i++)
{
short val = (short)(sineWave[i] + distribution(generator));
if(val >= 0 && val <= 256)
{
values[val * 1024 + i] += 1.0f;
max_value = fmax(values[val * 1024 + i], max_value);
}
}
static ImPlotAxisFlags axes_flags = ImPlotAxisFlags_LockMin | ImPlotAxisFlags_LockMax;
ImPlot::PushColormap(ImPlotColormap_Jet);
ImPlot::SetNextPlotLimits(0,1024, 0,256);
if(ImPlot::BeginPlot("##HeatmapTest", NULL, NULL, ImVec2(-1, -1), 0, axes_flags, axes_flags))
{
ImPlot::PlotHeatmap("heatTest", values, 256, 1024, 0.0f, max_value, NULL, ImPlotPoint(0,0), ImPlotPoint(1024,256));
ImPlot::EndPlot();
}
ImPlot::PopColormap();
ImGui::End();
} Where float sineWave[1024]{};
for(int i = 0; i < 1024; i++)
sineWave[i] += (110.0f * sin(2.0f * M_PI * (float)i / 512.0f) + 128.0f); This is the performance that I'm getting on my MacBook 2018 (Intel Core i5, 2.3 GHz; compiled using AppleClang 12 with As you can see, I get consistently 60 fps. It occasionally drops down to something like 59.7 but it rapidly goes up to 60. Also, I have the implot demo rendering behind this ImGui window and multiple applications opened, so I don't see why 60 fps would not be achievable. If you're not getting this performance it might be due to what @ozlb suggested, maybe the library is doing too many passes over the data. Or maybe you're compiling using less optimizations like Anyway, I hope this was somewhat helpful. If you need me to run any benchmarks, tests or anything, just let me know :) |
@marcizhu , I get similar performance with your code on my laptop, which has a GTX 2080, so this is obviously CPU bound. A quick profile revealed the main culprits to be @ozlb , yes there's a lot of looping in this implemenation. At the very least, I can merge |
@epezent Hmmm... I see no easy way to preserve the To be honest, I wouldn't try to "save" the constexpr Im32U ColorIm32(float a, float r, float g, float b)
{
Im32U color =
(((Im32U)(a * 255.0f + 0.5f)) << 24) |
(((Im32U)(r * 255.0f + 0.5f)) << 16) |
(((Im32U)(g * 255.0f + 0.5f)) << 8) |
(((Im32U)(b * 255.0f + 0.5f)) << 0);
return color;
} |
I've been working on this the past few days...finally wrapped up my PhD, so I have more free time now :)
@marcizhu, using all of the optimizations above I am able to render your example code above at 105 FPS vs the previous 60 FPS. I believe there may still be some gains to be had, but overall I'm happy with this performance. I would appreciate it if you could pull this PR and report back with any gains you see and/or bugs you find. Please try different combinations of the two compile time options I mentioned. If both of you could provide feedback on the API and signatures for |
Looping in @jvannugteren. The changes to heatmap and colormaps are relevant to your application. If you'd be willing to test this PR, it'd be much appreciated! |
congrats! 💯 |
First of all, apologies for my late reply. Due to timezone differences and university I was unable to do the performance tests until now.
Congrats!! 😄
Sure thing! I've done some performance tests and using both flags. Here's a small table with my results (as always, ymmv):
I am more than happy with these changes. The performance has greatly improved and now ImPlot isn't CPU-bound as it used to be (the macOS performance monitor shows a 70% CPU and around 30% GPU usage for the small example instead of the previous 100% CPU and 10% GPU). I'm sure I could optimize my code a lot more and have even better performance but still, over 100 FPS is an impressive result if we take into account the incredibly big size of the heatmap (1024x256, over a quarter of a million squares redrawn 100 times per second!).
The new API looks great to me: it's easy to use, yet it's powerful and really flexible. I can't suggest any changes, it looks pretty much perfect to me! 😄 |
Thanks Marc! Thorough as always. Glad to see the performance gains here. I was leaning toward keeping |
Personally, I think that ~100kB is not much nowadays. Since this project is not aimed at embedded systems, where resources like RAM might be limited, we could easily leave the flag enabled by default so that the library by default offers its best performance unless the user decides to reduce memory footprint in exchange for a bit worse performance. |
I've tested the PR for my application and encountered no problems with both compile flags enabled. Also got pretty good performance increase. |
Today I made an interesting mistake, probably during copy pasting, which took me a couple of hours to find. I was experiencing vague segfaults and "double linked list corruption", which occured after leaving the application open for a while. In the end it came down to the following lines of code:
The ImPlotColormap_Paired input in the PopColormap is obviously the problem. This compiles fine as PopColormap takes an integer as input. However, the result is that the colormap attempted to be popped 3 times. As a suggestion maybe you can add a user assert to see if the size of gp.ColormapModifiers is going to fall below zero. |
Probably not a bad idea. I don't believe ImGui does this check for its pops, but there's no reason we cant. |
No description provided.