From the mailing list:
From j.witteveen@gmail.com
Bug #1 with patch
When a factor name is a prefix of another, gnuplot treats them as the same.
Hence a datafile with
0 0 0 foobar
0 1 0 foo
will lead to only one boxplot.
Bug #2 with patch
When a row in the datafile contains no factor, its y-value is stored in
place of the factor identifier, leading to ambiguities.
If the datafile contains
0 0 0 foo
0 1
The second row will be marked as part of the 'foo' boxplot.
Bug #3 with patch
The x-value of individual boxplots is ignored when placing tics for
multiple boxplots.
This patch fixes that issue. It requires the previous patch, which
introduces default factor identifiers. With this patch in place, there
is a case for changing the default separation value for the boxplot
style to 0.
Patch #4
With a default factor identifier, the expensive filter_boxplot_factor
routine can be made obsolete. This leads to more efficient plotting of
boxplots.
Comments:
Bug #1 - yes, applied to CVS for 5.0 and 5.1
Bug #2 - I agree there may be a bug here, but I can't reproduce exactly what you describe. Could you please provide a full example with both the input and the command used to plot?
If I understand the problem case correctly, a missing column in the input data is a failure in its own right and the point should be discarded before ever reaching your new code. The actual result seems to depend on additional factors like "set datafile separater". But in cases where the point is not discarded I don't see it incorrectly added to some other category - instead the plot contains an extra boxplot column with a blank label into which the problem points are placed. That's arguably a correct result, although it's not what I expected.
Bug #3 - Hmm. I was never happy with the x-coordinate field. Surely it's nonsense to assign an X coordinate to each data point. I'll look at this more closely.
Bug #4 - I'll defer looking at this one until #2 and #3 are sorted out.
Bug #2 -
I've now applied this patch to both 5.0 and 5.1 with 2 modifications.
1) It handles the case of the factor column being present but empty
(e.g. in a *.csv file) as well as the case of the column being
missing altogether.
2) The factor is now stored in the z/w/misc slot of the point structure
rather than misusing ylow. For the moment it is also stored in
ylow in the case of true 4-column input since that is expected by
the sorting and drawing routines. But this can go away later, as
shown by your patch #4.
Disregarding patch #3 for now since it addressing a slightly different
issue, it would be nice if you would revise patch #4 so that it
(1) does not refer to or use the changes in patch #3
(2) gets rid of all the y value mangling, which is no longer necessary.
Instead of temporarily replacing y with VERYLARGE / UNDEFINED so that
the point count can be based on y, simply step past any points for
with the factor (now stored in z) doesn't match.
Bug #2 - There are really two bugs here. Lets look at the following datafile (tests conducted with 4.6 patchlevel 6)
---data---
0 10 0 foo
0 10 0 bar
0 0 0
0 1
plotted with
plot 'data' with boxplotThe first two lines are fine. At the third line the check_or_add_boxplot_factor gets a NULL string and returns 0, the factor index of the first factor, hence adding the point to the 'foo' boxplot. At the third line, the point is added with an ylow value of 1, the factor index of the second factor, hence adding the point to the 'bar' boxplot.
Bug #3 - This bug initiated my coding spree as I had encountered it too often by now. It was independently reported earlier: https://sourceforge.net/p/gnuplot/mailman/message/29174329/. While it may appear nonsense to store an X coordinate for every data point, it is not nonsense to have an X coordinate for every data point in your data file. In particular when you factorize by the column holding this coordinate you nicely get boxplots for each of your X values. My patch is the simplest backward compatible fix I could think of.
Patch #4 - The heart of the patch is sorting the data by factor identifier and forcing UNDEFINED points to the end of the list in one go. It already does away with all other y value mangling. I don't think it really depends on anything in patch #3, although I feel that patch #4 is of lesser importance than patch #3. I can rewrite patch #4 to make use of the z value.
Bug #2:
The behavior is different in version 5 because in the number of input columns expected is determined only once, at the beginning of the file input, rather than being reevaluated for every input line. Since there are 4 columns of data in the first line of your test case, in version 5 the command
gnuplot> plot 'data' with boxplot
is implicitly treated as
gnuplot> plot 'data' using 1:2:3:4 with boxplot
The 3rd and 4th lines of input data must be handled as error conditions because there are columns missing.
Arguably the "with boxplot" option should require an explicit "using" spec, but it's a bit late to change that.
As noted before, there is yet another wrinkle in both versions 4 and 5 if the input data is from a csv file (e.g. "set datafile separator comma").
Anyhow I think that the patched version now in CVS for version 5 handles these input cases correctly. Please test.
Patch #4:
I just meant that the patch itself can only be applied on top of your patch #3. This got in the way of my testing it separately.
Bug #3:
I honestly did not understand the earlier bug report you refer to, and similarly I am not sure I understand the problem you are addressing in your patch #3. I had put that aside to look at after dealing with the other issues.
Could you clarify exactly what the use case or expectation is that is causing a problem? I can understand wanting to give an explicit ordering if there are multiple boxplots on a graph. The boxplot.dem gives an example of doing this. But I don't see why an explicit x coordinate is relevant. Is the idea that giving x coordinates would provide another way of reordering the categories (a.k.a. factors)?
Or is this aiming towards a more complex use like superimposing a boxplot on top of some underlying chart or map? But in that case I think you would want to specify both x and y.
Revised code equivalent to your patch #4 is now in cvs for 5.0 and 5.1
I still haven't looked at Bug #3
Bug #3 - Consider the following datafile (tests conducted with 4.6 patchlevel 6)
plotted with
Expected result: points at (2,1), (3,2), (5,3) and labels at x coordinates 2, 3, 5.
Actual result: points at the correct places, all labels at x coordinate 2.
Patch #4 - The commited code is not at all equivalent.
Old situation:
After patch #4:
Your 'equivalent' code does not reduce the complexity of either the code or the running time as much as patch #4. After patch #4 the factors are treated as different boxplots stored sequentially in the points array.
I do not mind defending my patch #4, but I am curious about your concerns. It reduces the code count and call graph while yielding faster run-times. What is there not to like?
I agree that storing factors in the z coordinate is an improvement.
As an aside: it would be nice if gnuplot used a more modern source code management system. It would facilitate contribution and attribution.
Last edit: Jouke Witteveen 2015-01-06
I might be missing something clever, but so far as I can see the only difference between your patch #4 and the code I committed is that your patch assumes a known ordering of P categories within the complete set of data and tracks the start/end indices for each. Then it does P passes in which only the subset of data between start/end indices are sorted. The CVS code does not assume a known ordering. It also does P passes with a sort in each pass, but each pass uses the current category index as a primary key in the sort while sorting the entire array. Both approaches require an initial sort of the full data set.
So yes, for large numbers of data points there is a difference in run time.
As built here the code size for the CVS version is smaller than for 5.0.0 + your 4 patches, but the difference is trivial. I could add back your clever idea to sort subsets of the data in place. It's not a big deal and not a lot of code.
However if the sort time is a major concern then it would be faster yet to filter the categories during input so that a sort over the full set of data is not required at all. E.g.
plot for [i=1:P] 'data' using (i):2:3:($4==i ? i : NaN) with boxplot
The only downside here is that the data is re-read from disk multiple times. But I assume that if the sort time is a concern at all, it must be slow enough to dominate the data input speed. Is that wrong in practice?
===
The whole idea of internally sorting category labels as integer "factors" indexing a list of strings strikes me as an ugly hack, particularly since the only sort order offered is alphabetic. I had never really looked at this part of the code before. Wouldn't it be more natural to define a function X(i) at the command line level so that the placement of the "factor" data is controlled by
plot for [i=1:P] 'data' using (X($4)):2:3:4 with boxplot
Then all the internal compare_boxplot_factors sorting code and permutation arrays could go away. I think your Bug #3 assumes that this would work, but I'm not sure that was the original intent of the code that now exists.
===
This brings us back to your concern in Bug #3 (sorry - I still haven't looked at it in detail). I think it best to take discussion of that back to the gnuplot-beta list, since I honestly don't know what the best fix is. It is utterly unclear to me how to combine the existence of an explicit x coordinate in column 1 with the "set style boxplot separation <foo>" style option. They seem contradictory in intent. I think Peter Juhasz was the original author; let's see if he can clarify the intent.</foo>
While I see nothing wrong with your analysis, I still think patch #4 yields a more elegant code base and it feels awkward to resort all data every time when it is only needed once. Currently there is a complex comparison function which uses a global variable and the plotting routine does not have simple pre- and postconditions. With patch #4 you start with a points array containing the data for P boxplots sequentially in the order that they have to be plotted and you end up with the same data, but sorted per boxplot. If you re-run the plotting routine on the same plot variable, all sorting is trivial and you'll be done quickly. Of course, this is all theoretical and not of great practical relevance. I might have been looking at the code for too long, but my patch #4 just strikes me as more elegant than the current code.
===
Yes, it is all a bit hacky, but I started this work when 5.0 was about to be released and my fix for bug #3 is backward compatible and I guess that is important at this stage. An entirely different implementation of boxplots might be desirable (although given the data structures used in gnuplot you might up with something not at all fundamentally different), but that is no reason not to take on this fix for now.
If I were to suggest an incompatible redesign, I would drop the separation variable and interpret the fourth column as labels for the x coordinates:
where w and l are optional and the boxplots are grouped by and plotted at their x coordinate.
It appears I was indeed missing something clever. It is sufficient to sort the data only once, using a 2-key sort (factor index first, y value second). No further sorting is necessary. Also it is not necessary to sort the factor labels in a separate step, as the initial list can be constructed using an insertion sort. The version now in CVS combines the approach your patch #4 and my initial effort.