Need an advice

[Tony Yu]

On Fri, Dec 30, 2011 at 5:48 PM, Ralf Gommers
<ralf.gommers at googlemail.com>wrote:

>
>
> On Fri, Dec 30, 2011 at 8:05 PM, Tony Yu <tsyu80 at gmail.com> wrote:
>
>>
>>
>> On Fri, Dec 30, 2011 at 1:19 PM, Ralf Gommers <
>> ralf.gommers at googlemail.com> wrote:
>>
>>>
>>>
>>> On Fri, Dec 30, 2011 at 5:46 PM, Tony Yu <tsyu80 at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On Fri, Dec 30, 2011 at 8:20 AM, Nadav Horesh <nadavh.horesh at gmail.com>wrote:
>>>>
>>>>> Remarks for the convolution subpackage:
>>>>> 1. The __init__.py should be probably changed to
>>>>>
>>>>> from ext import pyconvolve as convolve
>>>>>
>>>>
>>>> Agreed. Alternatively, the c-method could be renamed so that the python
>>>> method in ext can be named convolve.
>>>>
>>>>>
>>>>> 2. The test_convolution.py file is very instructive. It should be
>>>>> polished a bit. I think it would be worthwhile to add also
>>>>> scipy.signal.sepfir2d and cv2.sepFilter2D to the test.
>>>>>
>>>>
>>>>> 3. I found the convolve speed in par with cv.convolve2d (it can be
>>>>> slower or faster depending on the kernel)
>>>>>
>>>>>    Nadav
>>>>>
>>>>
>>>> Thanks for testing the code.
>>>>
>>>> I've got the code compiling on my system now, so I was able to clean up
>>>> a few things. Could you update to what's currently on my
>>>> skimage-convolution branch to check that it builds on your system? If not,
>>>> what flags do you have to change/add to get it to compile?
>>>>
>>>> -Tony
>>>>
>>>> P.S. If any one else is building this on OS X: ppc compatible builds is
>>>> turned on by default (at least on my system). This should be turned off
>>>> before building. Add the following line to ~/.profile, or just run it right
>>>> before building:
>>>>
>>>> export ARCH_FLAGS="-arch i386 -arch x86_64"
>>>>
>>>
>>> Can you explain why is this necessary?
>>>
>>> In convolution/setup.py I also see you add "-ffast-math", which is a
>>> GCC-specific flag. It's better to remove it I think, it may give problems
>>> with other compilers.
>>>
>>> Ralf
>>>
>>
>> On my system at least, running gcc will, by default, add "-arch ppc"
>> along with i386 and x86_64. As far as I can tell, PPC doesn't support SSE,
>> which is used by this implementation.
>>
>
> That's true. I can't tell for sure, but it seems that this code is not
> optional. Meaning PPC is unsupported, and the ARCH_FLAGS need to be defined
> on OS X. I'm only a very occasional user now so it doesn't bother me too
> much, but isn't this a large penalty for the benefit you get?
>

Well, I assume that we would be able to add a check to disable the build on
unsupported architectures. As far as, whether it's worth it: I don't know.
Is PPC very common? I'm only familiar with it in the context of old Apple
computers. From the Wikipedia page <http://en.wikipedia.org/wiki/PowerPC>,
it seems like PPC is mainly used in servers now.

As for the benefit: I think getting close to the performance of OpenCV is
pretty significant. (On my system, however, I don't quite see the
performance boost that Nadav sees: skimage: 0.034 secs, ndimage: 0.089
secs, opencv: 0.018 secs, for the output of
'test_convolution.py'---nevertheless, it's a significant improvement over
ndimage). And, in comparison, OpenCV can be incredibly difficult to build.

There are probably other ways to get similar performance, but like I said,
I don't do any serious non-Python coding, so I have no idea what those
alternatives may be. Stefan and Pieter (the original author of this code)
have probably thought this through a bit more.

-Tony


>> The flag is left over from the original implementation. Removing
>> "-ffast-math" works fine on my system. To be honest, I don't do much
>> non-Python coding so I don't know the implications of adding/removing the
>> flag.
>>
>
> It's very likely to give problems with unusual (or even Intel) compilers I
> think.
>
> Ralf
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scikit-image/attachments/20111231/ebdbab0e/attachment.html>