I'm going to again defer the real question of the functionality and interface etc. to Eric and/or John etc., though it seems good in general.
I just have a couple of follow on comments related to implementation details below. Grégory Lielens wrote: > std::vector would have been nice, but I tried to respect the original > coding style, maybe this could be changed but I guess the original > author should have something to say about it.... > I think we want to use std::vector where possible for new code, and convert as we go through old code. Manual malloc/free is just too error prone. (We're forced to do some by interfacing with Python, but it should be kept to a minimum). Maybe this should be added to the coding guidelines? > Only thing I did not change is the allocation of acols/arows even when > they are not used. It is not difficult to do, but the code will be a > little more complex and I think that the extra memory used is not > significant: The memory for those mapping structure is doubled (1 float > and 1 int vector of size N, instead of a single int vector of size N), > but those mapping structures are an order of magnitude smaller than the > image buffer of size N*N of 4 char anyway... > > If one agree that this is to be saved at the (slight) expense of code > conciseness/simplicity, I can add this optimisation... > I don't understand the complexity. Isn't it as simple as moving: acols = reinterpret_cast<float*>(PyMem_Malloc(sizeof(float)*cols)); (and the following NULL check) inside of the "if (interpolation == Image::BILINEAR)" block? Heap allocations can be expensive, so it would be great to minimize them. > Thanks for pointing the error at the left/bottom pixel lines, it is > corrected now :-) > Great! > And I set interpolation to NEAREST by default, so that the behavior of > NonUniformImage remains the same as before if the user do not specify > otherwise (I forgot to do it in the first patch)... > Sounds reasonable -- though perhaps we want to be consistent instead with the standard (uniform) images, which default to the "image.interpolation" setting in matplotlibrc. Of course, "image.interpolation" has a bunch of options that nonuniform doesn't currently support... What do others think? Cheers, Mike > > I include the new patch, > > Best regards, > > Greg. > > On Fri, 2008-08-15 at 15:45 -0400, Michael Droettboom wrote: > >> Thanks for all the work you put into this patch. >> >> As you say, it would be nice to have a generic framework for this so >> that new types of interpolation could be easily added and to be able to >> support arbitrary (non-axis-aligned) quadmeshes as well. But that's >> even more work -- if we keep waiting for everything we want, we'll never >> get it... ;) I agree that Agg probably won't be much help with that. >> >> There are a couple of comments with the patch as it stands --> >> >> There seems to be a gap extrapolating over the left and bottom edge (see >> attached screenshot from pcolor_nonuniform.py). >> >> Memory management looks problematic, some of which I think you inherited >> from earlier code. For example, arows and acols are never freed. >> Personally, I think these temporary buffers should be std::vector's so >> they'll be free'd automatically when scope is left. It might also be >> nice to move all of the Py_XDECREF's that happen when exceptions are >> thrown to either a master try/catch block or an "exit" goto label at the >> bottom. The amount of duplication and care required to ensure >> everything will be freed by all of the different exit paths is a little >> cumbersome. >> >> Also, acols and arows are only used in BILINEAR interpolation, but they >> are allocated always. >> >> Once these issues are addressed, it would be great to have someone who >> *uses* the nonuniform pcolor functionality (Eric Firing?) have a look at >> this patch for any regressions etc.. Assuming none, I'll be happy to >> commit it (but I won't be around for a week or so). >> >> Cheers, >> Mike >> >> Grégory Lielens wrote: >> >>> Hi all, >>> >>> here is a patch which implement bilinear interpolation on irregular grid >>> ( i.e. it allows NonUniformImage >>> to accept both 'nearest' and 'bilinear' interpoaltion, instead of only >>> 'nearest'.) >>> >>> It is not perfect, given the current architecture of the image module I >>> think there is not simple way >>> to specify other interpolations (except for 'nearest', all >>> interpolations are implemented at the AGG level, not in >>> matplotlib itself). >>> For the same reason, it is not possible to interpolate before colormap >>> lookup instead of after (and this >>> is usually where the biggest effect is, when dealing with coarse grid). >>> However, I think it is already quite useful and the best one ca do >>> without a -very- extensive rewrite >>> of the matrix map modules.... >>> >>> BTW, I think it also corrects a small bug in the 'nearest' >>> interpolation: the last intervals was ignored >>> in the CVS version, now it is taken into account. >>> >>> BOTH nearest and bilinear interpolation do the same for values oustside >>> the data grid: they just use boundary values (constant extrapolation). >>> I avoided linear extrapolation for 'bilinear', as extrapolation is >>> usually dangerous or meaningless (we can go outside the colormap very >>> fast)... >>> >>> I have included a small example showing how both interpolation works.... >>> >>> Any remarks, could this be added before the next release? ;-) >>> >>> >>> Greg. >>> >>> >>> >>> >>> >>> On Mon, 2008-08-11 at 16:50 +0200, Grégory Lielens wrote: >>> >>> >>>> On Fri, 2008-08-08 at 16:05 +0200, Grégory Lielens wrote: >>>> >>>> >>>>> Hello everybody, >>>>> >>>>> I have sent this message to the user group, but thinking of it, it may be >>>>> more >>>>> relevant to the development mailing list...so here it is again. >>>>> >>>>> >>>>> >>>>> We are looking for the best way to plot a waterfall diagram in >>>>> Matplotlib. The 2 functions which could be used >>>>> to do that are (as far as I have found) imshow and pcolormesh. Here is a >>>>> small script that use both to compare the output: >>>>> >>>>> ----------------- >>>>> >>>>> from pylab import * >>>>> >>>>> >>>>> delta = 0.2 >>>>> x = arange(-3.0, 3.0, delta) >>>>> y = arange(-2.0, 2.0, delta) >>>>> X, Y = meshgrid(x, y) >>>>> Z1 = bivariate_normal(X, Y, 1.0, 1.0, 0.0, 0.0) >>>>> Z2 = bivariate_normal(X, Y, 1.5, 0.5, 1, 1) >>>>> # difference of Gaussians >>>>> Z = 10.0 * (Z2 - Z1) >>>>> figure(1) >>>>> im = imshow(Z,extent=(-3,3,-2,2)) >>>>> CS = contour(X, -Y, Z, 6, >>>>> colors='k', # negative contours will be dashed by default >>>>> ) >>>>> clabel(CS, fontsize=9, inline=1) >>>>> title('Using imshow') >>>>> figure(2) >>>>> im = pcolormesh(X,-Y,Z) >>>>> CS = contour(X, -Y, Z, 6, >>>>> colors='k', # negative contours will be dashed by default >>>>> ) >>>>> clabel(CS, fontsize=9, inline=1) >>>>> title('Using pcolormesh') >>>>> show() >>>>> >>>>> --------------------- >>>>> >>>>> >>>>> The problem is that we need some of the flexibility of pcolormesh (which >>>>> is able to map the matrix of value on any deformed mesh), while >>>>> we would like to use the interpolations available in imshow (which >>>>> explain why the imshow version is much "smoother" than the pcolormesh >>>>> one). >>>>> >>>>> In fact, what would be needed is not the full flexibility of pcolormesh >>>>> (which can map the grid to any kind of shape), we "only" have to deal >>>>> with rectangular grids where x- and y- graduations are irregularly spaced. >>>>> >>>>> Is there a drawing function in Matplotlib which would be able to work >>>>> with such a rectangular non-uniform grid? >>>>> And if not (and a quick look at the example and the code make me think >>>>> that indeed the capability is currently not present), >>>>> what about an extension of imshow which would work as this: >>>>> >>>>> im = imshow(Z,x_gridpos=x, y_gridpos=y) #specify the >>>>> position of the grid's nodes, instead of giving the extend and assuming >>>>> uniform spacing. >>>>> >>>>> Longer term, would a pcolormesh accepting interpolation be possible? The >>>>> current behavior, averaging the color of the grids node to get a uniform >>>>> cell color, >>>>> is quite rough except for a large number of cells...And even then, it >>>>> soon shows when you zoom in... >>>>> >>>>> The best would be to allow the same interpolations as in imshow (or a >>>>> subset of it), and also allows to use interpolation before colormap >>>>> lookup (or after), >>>>> like in Matlab. Indeed, Matlab allows to finely tune interpolation by >>>>> specifying Gouraud (interpolation after color >>>>> lookup)/Phong(interpolation before color lookup, i.e. for each pixel). >>>>> Phong is usually much better but also more CPU intensive. Phong is >>>>> especially when using discrete colormap, producing banded colors >>>>> equivalent to countour lines, while Gouraud does not work in those >>>>> cases. >>>>> >>>>> Of course, the performance will be impacted by some of those >>>>> interpolation options, which would degrade performance in animations for >>>>> example.... but I think that having the different options available >>>>> would be very useful, it allows to have the highest map quality, or have >>>>> a "quick and dirty" map depending on situation (grid spacing, type of >>>>> map, animation or not, ...). >>>>> >>>>> Best regards, >>>>> >>>>> Greg. >>>>> >>>>> >>>> I have found a method which implement the proposed extension to imshow: >>>> NonUniformImage... >>>> >>>> However, this image instance support only nearest neighbor >>>> interpolation. Trying to set the interpolation (using the >>>> set_interpolation method) >>>> to something akin imshow throw a "NotImplementedError: Only nearest >>>> neighbor supported" exception.... >>>> >>>> So basically I am still stuck, it seems that currently there is no way >>>> in matplotlib to plot interpolated >>>> colormap on irregular rectangular grid, and even less on arbitrarily >>>> mapped grid... >>>> >>>> Is there any plans to add support for more interpolation in >>>> NonUniformImage in the future? Or maybe there is another >>>> drawing function that I did not find yet, with this ability? >>>> >>>> >>>> Best regards, >>>> >>>> Greg. >>>> >>>> ------------------------------------------------------------------------ >>>> >>>> ------------------------------------------------------------------------- >>>> This SF.Net email is sponsored by the Moblin Your Move Developer's >>>> challenge >>>> Build the coolest Linux based applications with Moblin SDK & win great >>>> prizes >>>> Grand prize is a trip for two to an Open Source event anywhere in the world >>>> http://moblin-contest.org/redirect.php?banner_id=100&url=/ >>>> ------------------------------------------------------------------------ >>>> >>>> _______________________________________________ >>>> Matplotlib-devel mailing list >>>> Matplotlib-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel