Mailman 3 January 2012 - scikit-image

Re: Harris corner detection (#106)
by Stéfan van der Walt Jan. 9, 2012

Jan. 9, 2012

On Mon, Jan 9, 2012 at 8:05 AM, Tony Yu <tsyu80(a)gmail.com> wrote: > You know what: I think I'm just going to merge your Harris detection pull > request as it is now, and then I'll start a new pull request with the peak > detection part factored out. I think that's the way to go. Peak detection would be very useful--it crops up all over the place. I was trying to recall who recently shared a review of peak detection algorithms with me, and I think it might have been you :) There are so many out there, almost all heuristics. Typically, there are two varieties: those that simply consider the input data (e.g., dividing the image into windows and choosing the maximum value in each), and those that in addition consider other peaks found (e.g., to avoid identifying very closely spaced peaks). The problem takes on an interesting twist when working with noisy data, where many points seem like peaks but aren't. Here, multi-level decompositions such as the Gaussian scale space or wavelets are often employed. Regards Stéfan

1 0

Re: Harris corner detection (#106)
by Tony Yu Jan. 9, 2012

Jan. 9, 2012

On Mon, Jan 9, 2012 at 4:36 AM, Nelle Varoquaux <nelle.varoquaux(a)gmail.com>wrote: > I think it is a good idea to do a such a method: maybe in the util module > ? On the other hand, the harris response by itself is totally useless, so I > suggest to keep it in "one chunk". > > N > Actually, I don't think this is much of a problem: if the peak detection part was factored out (maybe keeping it in the `feature` module instead of moving it to `util`), then you could just have the `harris` function call `_compute_harris_response` followed by a separate call to the peak detection function (possibly with different defaults that are tuned to the harris response array). I guess the question is whether this peak detection algorithm is generally useful, or if it's somehow specific to the harris response. From the looks of code, it seems the algorithm would be generally useful. You know what: I think I'm just going to merge your Harris detection pull request as it is now, and then I'll start a new pull request with the peak detection part factored out. -Tony > > > On 9 January 2012 09:49, Neil Yager <yager.neil(a)gmail.com> wrote: > >> I think this is a great idea. Peak detection comes up all the time >> (e.g. peaks in Fourier space), and a general public function would be >> very useful. >> >> Neil >> >> On Jan 6, 4:59 am, Tony Yu <tsy...(a)gmail.com> wrote: >> > On Thu, Jan 5, 2012 at 12:50 PM, Varoquaux < >> > >> reply+i-2636374-54829e76ead08234132b3dcaac8a1c3f1441e0cd-133...(a)reply.github.com >> > >> > > wrote: >> > > I fixed a few things, and added the parameter to set the deviation >> for the >> > > gaussian blur. I also added @GaelVaroquaux as a contributor, as he >> helped >> > > me debug the patch. >> > >> > > I'll now work on the panorama stitching example. >> > >> > I just read through the PR for Harris corner >> > detection<https://github.com/scikits-image/scikits-image/pull/106/files >> >. >> > I made some inline comments on github, but I wanted to raise a question >> the >> > list. >> > >> > It seems that everything in the `harris` function is essentially a peak >> > detection algorithm. (Please correct me if I'm wrong or if the >> algorithm is >> > specific to the Harris response.) It might be nice to make >> > `_compute_harris_response` a public function (maybe renamed to >> > `harris_response` or `harris`) and move the rest of `harris` to the >> > detection subpackage as a peak detection function. We could really use a >> > (or multiple) peak detection function(s) in skimage (e.g. with the hough >> > transform or template matching). >> > >> > -Tony >> > >

1 0

Re: Harris corner detection (#106)
by Neil Yager Jan. 9, 2012

Jan. 9, 2012

I think this is a great idea. Peak detection comes up all the time (e.g. peaks in Fourier space), and a general public function would be very useful. Neil On Jan 6, 4:59 am, Tony Yu <tsy...(a)gmail.com> wrote: > On Thu, Jan 5, 2012 at 12:50 PM, Varoquaux < > reply+i-2636374-54829e76ead08234132b3dcaac8a1c3f1441e0cd-133...(a)reply.github.com > > > wrote: > > I fixed a few things, and added the parameter to set the deviation for the > > gaussian blur. I also added @GaelVaroquaux as a contributor, as he helped > > me debug the patch. > > > I'll now work on the panorama stitching example. > > I just read through the PR for Harris corner > detection<https://github.com/scikits-image/scikits-image/pull/106/files>. > I made some inline comments on github, but I wanted to raise a question the > list. > > It seems that everything in the `harris` function is essentially a peak > detection algorithm. (Please correct me if I'm wrong or if the algorithm is > specific to the Harris response.) It might be nice to make > `_compute_harris_response` a public function (maybe renamed to > `harris_response` or `harris`) and move the rest of `harris` to the > detection subpackage as a peak detection function. We could really use a > (or multiple) peak detection function(s) in skimage (e.g. with the hough > transform or template matching). > > -Tony

2 1

Re: [scikits-image] Harris corner detection (#106)
by Tony Yu Jan. 6, 2012

Jan. 6, 2012

On Thu, Jan 5, 2012 at 12:50 PM, Varoquaux < reply+i-2636374-54829e76ead08234132b3dcaac8a1c3f1441e0cd-133031(a)reply.github.com > wrote: > I fixed a few things, and added the parameter to set the deviation for the > gaussian blur. I also added @GaelVaroquaux as a contributor, as he helped > me debug the patch. > > I'll now work on the panorama stitching example. > > I just read through the PR for Harris corner detection<https://github.com/scikits-image/scikits-image/pull/106/files>. I made some inline comments on github, but I wanted to raise a question the list. It seems that everything in the `harris` function is essentially a peak detection algorithm. (Please correct me if I'm wrong or if the algorithm is specific to the Harris response.) It might be nice to make `_compute_harris_response` a public function (maybe renamed to `harris_response` or `harris`) and move the rest of `harris` to the detection subpackage as a peak detection function. We could really use a (or multiple) peak detection function(s) in skimage (e.g. with the hough transform or template matching). -Tony

1 0

Re: Need an advice
by Ralf Gommers Jan. 2, 2012

Jan. 2, 2012

On Sat, Dec 31, 2011 at 6:51 AM, Tony Yu <tsyu80(a)gmail.com> wrote: > > On Fri, Dec 30, 2011 at 5:48 PM, Ralf Gommers <ralf.gommers(a)googlemail.com > > wrote: > >> >> >> On Fri, Dec 30, 2011 at 8:05 PM, Tony Yu <tsyu80(a)gmail.com> wrote: >> >>> >>> >>> On Fri, Dec 30, 2011 at 1:19 PM, Ralf Gommers < >>> ralf.gommers(a)googlemail.com> wrote: >>> >>>> >>>> >>>> On Fri, Dec 30, 2011 at 5:46 PM, Tony Yu <tsyu80(a)gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Fri, Dec 30, 2011 at 8:20 AM, Nadav Horesh <nadavh.horesh(a)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. > OS X on older PPC machines is still common enough, for example the laptop I build numpy/scipy binaries on has a PPC cpu. Also, even for many Intel based machines the Python defaults include PPC compilation, so this is going to trip people up. Therefore a fallback would be useful. Ralf 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 >> >> > >

1 0

Re: Need an advice
by Nadav Horesh Jan. 2, 2012

Jan. 2, 2012

Remarks after a test of the convolution from the recent git repository: 1. In the recent opencv (2.31a) there is no automatic conversion between numpy arrays to CV matrices. I attached a modification that makes the test run with both cv versions. 2. Shouldn't be a way to copy compilation flags from pythonx.x/config/Makefile? I think (not sure) that numpy does it. Nadav. 2011/12/31 Tony Yu <tsyu80(a)gmail.com> > > On Fri, Dec 30, 2011 at 5:48 PM, Ralf Gommers <ralf.gommers(a)googlemail.com > > wrote: > >> >> >> On Fri, Dec 30, 2011 at 8:05 PM, Tony Yu <tsyu80(a)gmail.com> wrote: >> >>> >>> >>> On Fri, Dec 30, 2011 at 1:19 PM, Ralf Gommers < >>> ralf.gommers(a)googlemail.com> wrote: >>> >>>> >>>> >>>> On Fri, Dec 30, 2011 at 5:46 PM, Tony Yu <tsyu80(a)gmail.com> wrote: >>>> >>>>> >>>>> >>>>> On Fri, Dec 30, 2011 at 8:20 AM, Nadav Horesh <nadavh.horesh(a)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 >> >> > >

1 0