Need an advice

[Ralf Gommers]

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


>
>
>
>>
>> 2011/12/28 Nadav Horesh <nadavh.horesh at gmail.com>
>>
>>> I compiled after setting the flags for a core2 linux machine.  A quick
>>> and dirty benchmark showed a x2 improvement over scipy.signal.sepfir2d.
>>> I think that are some declaration errors in the python code (for
>>> instance I think that the convolution __init__.py should be
>>>
>>> from ext import pyconvolve
>>>
>>> instead of
>>>
>>> from ext import convolve
>>>
>>>  I'll take a closer look later.
>>>
>>> Thank you,
>>>
>>>    Nadav.
>>>
>>>
>>>
>>> 2011/12/28 Tony Yu <tsyu80 at gmail.com>
>>>
>>>>
>>>>
>>>> On Tue, Dec 27, 2011 at 8:24 PM, Tony Yu <tsyu80 at gmail.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tue, Dec 27, 2011 at 8:29 AM, Nadav Horesh <nadavh.horesh at gmail.com
>>>>> > wrote:
>>>>>
>>>>>> How can I get it?
>>>>>>
>>>>>>   Nadav
>>>>>>
>>>>>
>>>>> The code Stefan mentioned is in an old pull request (PR #16<https://github.com/scikits-image/scikits-image/pull/16>).
>>>>> That PR is from before the switch from scikits.image to skimage. I've made
>>>>> a new branch<https://github.com/tonysyu/scikits-image/commits/skimage-convolution>in my git repo with the convolution code in the new namespace.
>>>>>
>>>>> If you're already running skimage from git, then you can use git to
>>>>> clone this branch:
>>>>>
>>>>> git remote -f add tonysyu git at github.com:tonysyu/scikits-image.git
>>>>> git checkout -b convolution tonysyu/skimage-convolution
>>>>>
>>>>>
>>>>> In case this isn't familiar: The first line adds my git repo to your
>>>>> list of remotes, and the "-f" flag fetches the tags from my repo (this
>>>>> ensures that you have the info about my branches). My repo is now added to
>>>>> your repo as a remote with the name "tonysyu" (you can change this). The
>>>>> second line creates a new branch "convolution", clones my
>>>>> "skimage-convolution" branch into it, and checks it out.
>>>>>
>>>>> Hope that helps,
>>>>> -Tony
>>>>>
>>>>
>>>> Oops, I forgot to mention: I added a compile flag that is specific to
>>>> my system (In convolution/setup.py there's an include flag '-I/usr/...')
>>>> that should probably be removed. Also, I should mention that I haven't
>>>> gotten this branch to compile on my system (OSX). I believe it should
>>>> compile OK on Linux (after removing the '-I/usr/...' compile flag).
>>>>
>>>> Good luck,
>>>> -Tony
>>>>
>>>>>
>>>>> 2011/12/23 Stéfan van der Walt <stefan at sun.ac.za>
>>>>>
>>>>>> On Thu, Dec 22, 2011 at 10:44 PM, Nadav Horesh <
>>>>>> nadavh.horesh at gmail.com> wrote:
>>>>>> > The application is not too sophisticated, and I do use numpy, scipy
>>>>>> and
>>>>>> > skimage for all I need. The only problem is speed, I need the
>>>>>> processing to
>>>>>> > be at l5-10 time faster. Convolutions take most of the processing
>>>>>> time. I
>>>>>> > prefer a flexible solution that would speed up also common
>>>>>> nonlinear filters
>>>>>> > (i.e. a median filter).
>>>>>>
>>>>>> We have a PR in the pipeline for doing really, really fast
>>>>>> convolutions... but it needs a bit of work.  Would you like to have a
>>>>>> look at it?
>>>>>>
>>>>>> Stéfan
>>>>>>
>>>>>
>>>>
>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scikit-image/attachments/20111230/bde59a45/attachment.html>