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

Reply via email to