bugGNU Octave - Bugs: bug #46570, Image package: wrong result with...

 
 

bug #46570: Image package: wrong result with edge

Submitted by:  Avinoam Kalma <avinoam>
Submitted on:  Sun 29 Nov 2015 08:39:00 PM UTC  
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  Carnë Draug <carandraug>
Originator Name:  Avinoam Open/Closed:  Closed
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Sun 31 Jul 2016 08:44:52 PM UTC, comment #15: 

Thanks a lot for all the fixes. Pushed without changes http://hg.code.sf.net/p/octave/image/rev/beb0382042e3

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Sun 31 Jul 2016 08:13:39 PM UTC, comment #14: 

To get the documentation into "sync" again, I have attached another PATCH, with V5 in its filename.

(file #38059)

Hartmut <hardy>
Sun 31 Jul 2016 06:59:45 PM UTC, comment #13: 

I think this leaves the documentation out of sync since they mention thresh being multiplied by different factors.

I have pushed http://hg.code.sf.net/p/octave/image/rev/ee99dba2cc5b after small editorial changes (using double quotes for strings, remove trailing whitespace, etc).  It will be part of the next minor release (2.6.0).

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Sun 31 Jul 2016 06:16:33 PM UTC, comment #12: 

Yes, the only difference between Prewitt, Sobel and Kirsch is the used edge finding matrix, supplied by fspecial. (Roberts has the additional difference of a slightly different auto threshold value and two additional return values.)

Carne, I've done the small performance change that you've mentioned in your last comment. And no, I wasn't intentionally coding for Nd images.

The accordingly changed PATCH will be attached as a file with V4 in its name.

(file #38058)

Hartmut <hardy>
Sun 31 Jul 2016 05:27:29 PM UTC, comment #11: 

So after this change, there's no difference between the sobel, prewitt, and kirsch methods?  Or am I missing something?

A small performance fix. In place operators reduce memory usage, so instead of:

  h1 = h1 ./ sum (sum (abs (h1)))

use:

  h1 ./= sum (sum (abs (h1)))

Also, use `sum (abs (x(:)))` (unless you really are coding for ND and want to normalize each plane by the sum of that plane, and on which case specify the dimension number as part of the call to sum).

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Sun 31 Jul 2016 03:35:23 PM UTC, comment #10: 

Here is the promised new PATCH  (the file with V3 in it). It should be applied on top of the current code version in the image repository.

A summary what this patch is supposed to do:

  • clarify calculation with squared quantities (Sobel/Prewitt/Kirsch, Roberts)
  • normalize the edge operator matrix (Sobel/Prewitt/Kirsch, Roberts)
  • adapt calculation of auto threshold (Sobel/Prewitt/Kirsch, Roberts)
  • keep edge strength before thinning step (Sobel/Prewitt/Kirsch, Roberts)
  • use imfilter instead of conv2 (Roberts)
  • add new tests for Prewitt and Roberts methods (Matlab compatible)

Please let me know if something is unclear or something misbehaves in this patch.

(file #38057)

Hartmut <hardy>
Fri 15 Jul 2016 11:37:26 PM UTC, comment #9: 

Yes, I can wait. If all goes well, I will make the new release either in August or early September.

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Fri 15 Jul 2016 11:39:40 AM UTC, comment #8: 

@Carne: Thank you for your feedback.

Those pushed changes seem wrong to me at some delicate points. If you can wait a couple of weeks, I will try to generate a patch to eventually fix this.

Hartmut <hardy>
Thu 14 Jul 2016 05:33:10 PM UTC, comment #7: 

The day you submit this patch, was also the day (well, 1 day difference) that edge was rewritten to reduce code duplication and got a load of new tests.  This meant that your patch didn't apply anymore but now that I did merge it, it automatically applied to all the other methods.

  http://hg.code.sf.net/p/octave/image/rev/87110623b042

I'm sorry this took such a long time.

(file #37874)

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Thu 10 Dec 2015 07:38:34 PM UTC, comment #6: 

Here is an slightly improved version of my last PATCH. (In the one before I forgot to implement the return value thresh.)

Its still only about the SOBEL method, but could easily extended to the other filter based edge methods.

(file #35690)

Hartmut <hardy>
Wed 09 Dec 2015 10:01:33 PM UTC, comment #5: 

Here is a new PATCH that includes all the discussed topics in this bug report, it also makes all the new tests for the Sobel method pass. The Sobel edge detection should be much more Matlab compatible now.

The patch currently only covers the SOBEL edge detection method. But the very same changes should also be applied to the

  • Prewitt
  • Roberts (maybe slightly different changes necessary?)
  • Kirsch (this one is not implemented in Matlab though)

edge detection methods.

Please

  • test this patch
  • if you have time: extend the changes of this patch also to the other three mentioned edge detection method.

(file #35681)

Hartmut <hardy>
Wed 02 Dec 2015 10:37:19 PM UTC, comment #4: 


>> im = ones(5,5);
>> im(3,3) = 0;
>> h1 = fspecial("sobel");
>> h3 = h1';

>> strength1 = sqrt( conv2(im, h1, "same").^2 + ...
                 conv2(im, h3, "same").^2 )
strength1 =

   4.24264   4.00000   4.00000   4.00000   4.24264
   4.00000   1.41421   2.00000   1.41421   4.00000
   4.00000   2.00000   0.00000   2.00000   4.00000
   4.00000   1.41421   2.00000   1.41421   4.00000
   4.24264   4.00000   4.00000   4.00000   4.24264

>> strength2 = sqrt( imfilter(im, h1, "replicate").^2 + ...
                 imfilter(im, h3, "replicate").^2 )
strength2 =

   0.00000   0.00000   0.00000   0.00000   0.00000
   0.00000   1.41421   2.00000   1.41421   0.00000
   0.00000   2.00000   0.00000   2.00000   0.00000
   0.00000   1.41421   2.00000   1.41421   0.00000
   0.00000   0.00000   0.00000   0.00000   0.00000

so conv2(im, h1, "same") gives spurious edges on the borders while imfilter(im, h1, "replicate") doesn't.
So it seems that imfilter is better than conv2.

imfilter with "replicate" has zero edges on the borders, which also seems to be good.
What do you think?

Avinoam Kalma <avinoam>
Project Member
Wed 02 Dec 2015 10:22:24 PM UTC, comment #3: 

I also think that using "imfilter(...,'replicate')" instead of the old "conv2" should help to make those edge methods (Robert, Sobel, Prewitt) more Matlab compatible, and also make the results really better. The current implementation over emphasises the image borders otherwise.

Hartmut <hardy>
Wed 02 Dec 2015 04:19:14 PM UTC, comment #2: 

Thank you for the tests. I have pushed them http://hg.code.sf.net/p/octave/image/rev/29d86b1f7930

Are you still investigating if your proposed fix on comment #0 is the right fix or should I push that as well?

Carnë Draug <carandraug>
Project MemberIn charge of this item.
Wed 02 Dec 2015 07:26:48 AM UTC, comment #1: 

Attached test file for Sobel filter.
Octave fails in most of them :-(

(file #35610)

Avinoam Kalma <avinoam>
Project Member
Sun 29 Nov 2015 08:39:00 PM UTC, original submission:  

Start working on BIST for edge, I have found this:

A = ones(5,5);
A(3,3) = 0;

B = edge(A)

B =

   0   0   0   0   0
   0   0   0   0   0
   0   0   0   0   0
   0   0   0   0   0
   0   0   0   0   0

the correct result is:

B =

     0     0     0     0     0
     0     0     1     0     0
     0     1     0     1     0
     0     0     1     0     0
     0     0     0     0     0

If we replace lines 175-176

        strength = sqrt( conv2(im, h1, "same").^2 + ...
                           conv2(im, h3, "same").^2 );

        strength = sqrt( imfilter(im, h1, "replicate").^2 + ...
                           imfilter(im, h3, "replicate").^2 );

We get the correct result, tough I do not know if this is the correct fix. The problem is that the borders get high values, so
the threshold is wrong.

BTW, this simple example can be served as first test for edge.

Avinoam Kalma <avinoam>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #38059:  edge_doc_strings_V5.patch added by hardy (3KiB - text/x-diff)
file #37874:  bug46570-harmut-edge-sobel-etc.cset added by carandraug (3KiB - application/octet-stream)
file #35610:  edge_test1.m added by avinoam (844B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by avinoam (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-31 hardy Attached File- => Added edge_doc_strings_V5.patch, #38059
    2016-07-31 carandraug StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2016-07-31 hardy Attached File- => Added FixCompatEdgePrewittRobertsSobelKirsch_V4.patch, #38058
    2016-07-31 hardy Attached File- => Added FixCompatEdgePrewittRobertsSobelKirsch_V3.patch, #38057
    2016-07-27 carandraug StatusFixed => In Progress
        Open/ClosedClosed => Open
    2016-07-14 carandraug Attached File- => Added bug46570-harmut-edge-sobel-etc.cset, #37874
        StatusPatch Submitted => Fixed
        Open/ClosedOpen => Closed
    2016-01-04 siko1056 StatusNone => Patch Submitted
        Assigned toNone => carandraug
    2015-12-10 hardy Attached File- => Added edge_sobel_compatibility_V2.patch, #35690
    2015-12-09 hardy Attached File- => Added edge_sobel_compatibility.patch, #35681
    2015-12-02 avinoam Attached File- => Added edge_test1.m, #35610

    Back to the top


    Powered by Savane 3.7