On Fri, Dec 30, 2011 at 5:46 PM, Tony Yu email@example.com wrote:
On Fri, Dec 30, 2011 at 8:20 AM, Nadav Horesh firstname.lastname@example.org:
Remarks for the convolution subpackage:
- 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.
- 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.
- I found the convolve speed in par with cv.convolve2d (it can be slower
or faster depending on the kernel)
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?
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.
2011/12/28 Nadav Horesh email@example.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
from ext import convolve
I'll take a closer look later.
2011/12/28 Tony Yu firstname.lastname@example.org
On Tue, Dec 27, 2011 at 8:24 PM, Tony Yu email@example.com wrote:
On Tue, Dec 27, 2011 at 8:29 AM, Nadav Horesh <firstname.lastname@example.org
How can I get it?
The code Stefan mentioned is in an old pull request (PR #16https://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 branchhttps://github.com/tonysyu/scikits-image/commits/skimage-convolutionin 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 email@example.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 firstname.lastname@example.org
On Thu, Dec 22, 2011 at 10:44 PM, Nadav Horesh < email@example.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?