matplotlib-devel Mailing List for matplotlib

I sent this message sometime on Friday but as it doesn't seem to have made it to the sourceforge list yet I'm assuming it's not going to and trying again.
On Thu, 2005年05月26日 at 21:15 -0500, John Hunter wrote:
> >>>>> "Nicholas" == Nicholas Young <su...@su...> writes:
> 
> Nicholas> Hi, I made a suggestion for improving imshow performance
> Nicholas> for plotting an image already in byte string form a
> Nicholas> while ago; some of the results are currently in CVS. I
> Nicholas> seems that other changes made to CVS at the same time or
> Nicholas> since mean that the floating point buffer source code is
> Nicholas> now much faster and I'd suggest taking anything sourced
> Nicholas> from my code back out.
> 
> Maybe that explains why I was never able to get the same performance
> benefits you were seeing.... But I was able to get 1.5x - 2.5x faster
> with your patch, which is pretty damned good. And the memory
> benefit of your patch could be substantial as well. Why pull it?
When I first tried the new CVS code (other than my own) I was rather
rushed trying to get a paper finished and tested with too small an image
and thus saw too small a speedup for it to be worth it. I'm also using
non-contiguous arrays which make the relative speed increase smaller. I
also didn't think the interface was particularly user friendly.
I'm now sure why my code was faster (a mixture of data copying using
optimised functions and no need do floating point to integer conversion)
and have written some new code which is slightly faster than the old and
has a nicer interface (patch to CVS attached).
> What are your latest profiling numbers?
Profiling with my latest version and comparing to CVS I get:
Running with 1024x1024:
Starting array:
 Array set up: resident stack size: 37408, size: 12571
 Tests done: resident stack size: 41536, size: 13596
 Elapsed: 7.674
Starting byte:
 Byte set up: resident stack size: 12876, size: 6429
 Tests done: resident stack size: 12880, size: 6428
 Elapsed: 0.521
Fractional improvement: 13.724
Running with 2048x2048:
Starting array:
 Array set up: resident stack size: 143956, size: 39197
 Tests done: resident stack size: 156252, size: 42269
 Elapsed: 29.577
Starting byte:
 Byte set up: resident stack size: 37464, size: 12573
 Tests done: resident stack size: 37464, size: 12572
 Elapsed: 2.098
Fractional improvement: 13.100
Running with 4096x4096:
Starting array:
 Array set up: resident stack size: 561756, size: 143646
 Tests done: resident stack size: 609388, size: 155963
 Elapsed: 127.401
Starting byte:
 Byte set up: resident stack size: 66376, size: 37178
 Tests done: resident stack size: 132280, size: 37177
 Elapsed: 8.526
Fractional improvement: 13.943
> Your patch came in just at the time I was leaving for Paris for a
> meeting, after which I experienced a rather nasty total hard drive
> failure that set me back in my mpl maintenance. I am not sure I am
> ready to give up on the ideas you introduced.... Can you remind me --
> did I manage to incorporate all of the matplotlib.axes and
> matplotlib.image patches I needed to take advantage of your patches to
> the _image extension code?
I think it's partly in there in a non-functional form, the patch I've
attached removes it and adds a new function to the Axes class called
directshow. This accepts the same syntax as imshow (where relevant)
rather than adding options to imshow; I chose to do this as my old
syntax for passing through imshow wasn't that easy to understand didn't
makes the different functionality clear. This function calls a class
call DirectImage which inherits from AxesImage.
I've also rewritten my c++ image object creation function called
frombyte to take an unsigned byte array as input rather than a buffer.
By using the std::memcopy function rather than a loop for copying the
speed advantage of passing data in as a buffer disappears and using
arrays is generally more intuitive. The function still only takes x*y*4
arrays as input at the moment as the processor time decrease from not
using loops to copy is fairly significant.
Nick

Eric Firing wrote:
> John,
>
> The attached diff contains a bugfix for the masking problem that Jeff 
> found:
>
> diff -Naur cntr.c_as_sent cntr.c > cntr_bugfix.diff
>
> where cntr.c_as_sent is the old version, cntr.c is the corrected version.
>
> I was simply not transferring the information correctly from the mask 
> to the "reg" array that cntr.c uses; I was setting a value, and then 
> accidentally overwriting it with what should have been an initialization.
>
> With this patch, masked arrays should be handled no better and no 
> worse than before the change from gcntr.c to cntr.c; as far as I know, 
> masking works perfectly for line contours, and it works correctly for 
> filled contours under many but not all circumstances. I tried two 
> workarounds for the latter problem:
>
> 1) in line 1359 of cntr.c, change the nchunk variable from 30 to a 
> much smaller value.
> long nchunk = 30; /* hardwired for now */
> This causes the generation of many small polygons, so it may slow 
> things down quite a bit, and it causes the boundaries to look a little 
> glitchy on the screen (gtkagg); but I haven't done timing tests, and I 
> haven't tried other backends.
>
> 2) change the end of ContourSupport._contour_args(), in contour.py:
>
> self.ax.set_xlim((ma.minimum(x), ma.maximum(x)))
> self.ax.set_ylim((ma.minimum(y), ma.maximum(y)))
> # Workaround for cntr.c bug wrt masked interior regions:
> #if filled:
> # z = ma.masked_array(z.filled(-1e38))
> # It's not clear this is any better than the original bug.
> return (x, y, z, lev)
>
> The workaround is to uncomment the "if filled:" block. This method 
> also works, but can leave some artifacts around the boundary of the 
> bad region.
>
> I am not recommending that either of these workarounds be added to 
> CVS, but if some individual runs into major trouble because of the 
> bug, then either of these methods could be used as a stopgap on an 
> individual basis.
>
> Eric
>
Eric: Thanks - contour indeed now seems to work perfectly with masked 
arrays, but I still have problems with contourf (see attached 
ortho_test.png). Unfortunately, neither of your suggested workarounds 
help, the first makes no difference and the second makes it much worse. 
-Jeff
-- 
Jeffrey S. Whitaker Phone : (303)497-6313
NOAA/OAR/CDC R/CDC1 FAX : (303)497-6449
325 Broadway Web : http://www.cdc.noaa.gov/~jsw
Boulder, CO, USA 80305-3328 Office: Skaggs Research Cntr 1D-124

John,
The attached diff contains a bugfix for the masking problem that Jeff found:
diff -Naur cntr.c_as_sent cntr.c > cntr_bugfix.diff
where cntr.c_as_sent is the old version, cntr.c is the corrected version.
I was simply not transferring the information correctly from the mask to 
the "reg" array that cntr.c uses; I was setting a value, and then 
accidentally overwriting it with what should have been an initialization.
With this patch, masked arrays should be handled no better and no worse 
than before the change from gcntr.c to cntr.c; as far as I know, masking 
works perfectly for line contours, and it works correctly for filled 
contours under many but not all circumstances. I tried two workarounds 
for the latter problem:
1) in line 1359 of cntr.c, change the nchunk variable from 30 to a much 
smaller value.
 long nchunk = 30; /* hardwired for now */
This causes the generation of many small polygons, so it may slow things 
down quite a bit, and it causes the boundaries to look a little glitchy 
on the screen (gtkagg); but I haven't done timing tests, and I haven't 
tried other backends.
2) change the end of ContourSupport._contour_args(), in contour.py:
 self.ax.set_xlim((ma.minimum(x), ma.maximum(x)))
 self.ax.set_ylim((ma.minimum(y), ma.maximum(y)))
 # Workaround for cntr.c bug wrt masked interior regions:
 #if filled:
 # z = ma.masked_array(z.filled(-1e38))
 # It's not clear this is any better than the original bug.
 return (x, y, z, lev)
The workaround is to uncomment the "if filled:" block. This method also 
 works, but can leave some artifacts around the boundary of the bad region.
I am not recommending that either of these workarounds be added to CVS, 
but if some individual runs into major trouble because of the bug, then 
either of these methods could be used as a stopgap on an individual basis.
Eric
John Hunter wrote:
>>>>>>"Jeff" == Jeff Whitaker <js...@fa...> writes:
> 
> 
> Jeff> The basemap toolkit relies on the 'badmask' parameter in
> Jeff> contour/contourf to mask out areas outside the desired map
> Jeff> projection region in matplotlib 0.80. It works quite
> Jeff> well. Unfortunately, the new masked array support in CVS
> Jeff> doesn't (I get all kinds of weird artifacts, especially for
> Jeff> contourf but also for contour). Can the old badmask
> Jeff> functionality be added back in for 0.81, at least
> Jeff> temporarily until the masked array support stabilizes?
> 
> I don't have anything to add to this, I'm just replying so I can CC
> Eric Firing, who added the masked array support to contour, because I
> don't know if he is on the devel list.
> 
> JDH