patchGNU Octave - Patches: patch #9407, image package: new function...

 
 

patch #9407: image package: new function houghpeaks.m

Submitter:  Hartmut <hardy>
Submitted:  Tue 11 Jul 2017 08:15:00 PM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 05 Nov 2018 10:13:42 PM UTC, comment #11: 
Avinoam Kalma <avinoam>
Group Member
Sun 04 Nov 2018 09:30:34 PM UTC, comment #10: 

The mentioned "bug report" in the mailing list (my comment #9) seems to be about the original (and copyrighted) Mathwork Matlab code to not work properly under Octave. So I tried to not look longer at this piece of code there.

Naturally this "bug report" should then have no relevance at all for the pushing of the Octave version of this function.

Hartmut <hardy>
Sun 04 Nov 2018 07:52:10 PM UTC, comment #9: 

There is a recent "bug report" in the mailing list on the code of this function: http://octave.1599824.n4.nabble.com/Help-td4689913.html

Should I find the time, I will look into this, if this points to any bug in the V5 code of houghpeaks.m .

Hartmut <hardy>
Sun 04 Nov 2018 07:42:08 PM UTC, comment #8: 

Yes, I still think that my houghpeaks.m (V5 from comment #6) is ready.

It doesn't harm to have another closer look at the code. But I think Carne and Avinoam also did some reviewing here already, more than a year ago.

Hartmut <hardy>
Sun 04 Nov 2018 09:14:31 AM UTC, comment #7: 

I woudld like to push this function.
Any comments?

Avinoam Kalma <avinoam>
Group Member
Wed 02 Aug 2017 06:12:11 PM UTC, comment #6: 

I have fixed the coding style issues mentioned in comment #5 (and originally in comment #3):

https://bitbucket.org/hgimpel/octave-image/commits/742179f9637703d17d8cabfc3f22a05a3fead223

And I will also attach the new version of houghpeaks.m (V5) as a file.

Is this new function houghpeaks.m now ready for inclusion into the image package?

(file #41393)

Hartmut <hardy>
Tue 01 Aug 2017 08:12:48 PM UTC, comment #5: 


> 3. Coding style: I have two questions:
>
> if isempty (threshold)
> or
> if (isempty (threshold))
>
> !isimage (H)
> or
> ! isimage (H)


It is:


## conditions in if/while are enclosed in parentheses
if (isempty (threshold))

## there is a space after the ! operator
! isimage (H)


These coding guidelines are inherited from Octave core.

Carnë Draug <carandraug>
Group Member
Tue 01 Aug 2017 07:57:39 PM UTC, comment #4: 

Thanks for your review, Avinoam:

1. good

2. thanks

3. I will leave those coding style issues to Carne. The coding guideline of the image package (in the file HACKING) doesn't say anything about this.

4. Yes, hough.m is still slightly Matlab incompatible. But I double checked, and it turned out that the different results you have observed here, ARE mostly really due to houghpeaks.m and not due to hough.m! Further investigation (using a saved csv file with the Matlab H result of peaks.m to debug the Octave function) showed that Matlab deletes the (593,136) value of H, therefore it gives the third peak (at 310,46) instead of the second one that Octave gives. This lead to the conclusion that Matlab uses a slightly bigger nhood for the peak deletion as my current Octave implementation. I am pretty sure that my current Octave implementation is doing what the Matlab help page says (size H = 723,180 -> divided by 50 gives 14.46 and 3.6 -> next bigger odd integer is 15 and 5. But Matlab seems to use 17 and 5.) So I have created a new version of my houghpeaks.m that also increases the nhood by 1 for half of the possible size values. (So their documentation is wrong here, or it is a Matlab bug I would say.) Nevertheless this new version of houghpeaks.m is now more Matlab compatible. All test still pass, Avinoams example from comment #3 now also gives Matlab compatible results, and this is added as a new test. (Slight differences of +-1 in the peak positions are really due to  hough.m and not to houghpeaks.m).

The result of this houghpeaks.m on Avinoams example in comment #3 now is:
P =

   586   136
   311    46
   596   136
   523   104
   374    46

But if you use the Matlab result of hough(I) (e.g. via csv export) as input to this Octave function houghpeaks.m then the result is 100% Matlab compatible, now.

You can find my new version of houghpeaks.m here:

https://bitbucket.org/hgimpel/octave-image/commits/7f928414c980bdd7360f937f32bad265011f24b5

and also attached (as file V4) to this bug report.

(file #41385)

Hartmut <hardy>
Mon 31 Jul 2017 04:43:24 AM UTC, comment #3: 

Sorry for my late response:

I have checked houghpeaks on Windows 7. A few points:

1. Testing:


>> test houghpeaks
PASSES 32 out of 32 tests


2. Nice demo :-)

3. Coding style: I have two questions:

  if isempty (threshold)
or
  if (isempty (threshold))

  !isimage (H)
or
  ! isimage (H)

4. Matlab Compatibility:


I = max(0, phantom ());
H = hough(I);
P = houghpeaks(H,5)


Octave
P =

   586   136
   594   136
   311    46
   523   104
   374    46

Matlab

P =

   585   136
   310    46
   595   136
   522   104
   373    46

The order is different, and there a shift of one pixel in Y.
The problem is probably from hough, and not from houghpeaks.

Thanks!




Avinoam Kalma <avinoam>
Group Member
Mon 17 Jul 2017 07:19:09 AM UTC, comment #2: 
Hartmut <hardy>
Wed 12 Jul 2017 07:41:31 PM UTC, comment #1: 

I've fixed a small bug in this new function. You can find the new version here:

https://bitbucket.org/hgimpel/octave-image/commits/bf7a64f34c358ecd9c1f64e4d1351909d767de72

I will also attach this new version as file "houghpeaks_V2.m".

(file #41194)

Hartmut <hardy>
Tue 11 Jul 2017 08:15:00 PM UTC, original submission:  

I have implemented the missing function "houghpeaks.m" for the image package.

This function finds peaks in the (linear) hough transfrom matrix, as returned by hough.m. The algorithm is taken from the book (Gonzalez,Woods,Eddins) that Matlab mentions on its help page of houghpeaks.

You can find the new file on my bitbucket repository:

https://bitbucket.org/hgimpel/octave-image/commits/4cdb27d091dbd33232ef5bc25750090054304266

The file is on the default branch there, in a new head, with the bookmark "new-houghpeaks" attached.

I will also add the function file (houghpeaks_V1.m) to this report.

I would be happy to hear comments or feedback on this.

Hartmut <hardy>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #41393:  houghpeaks_V5.m added by hardy (11KiB - text/x-objcsrc)
file #41385:  houghpeaks_V4.m added by hardy (11KiB - text/x-objcsrc)
file #41218:  houghpeaks_V3.m added by hardy (11KiB - text/x-objcsrc)
file #41194:  houghpeaks_V2.m added by hardy (11KiB - text/x-objcsrc)
file #41178:  houghpeaks_V1.m added by hardy (10KiB - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by hardy (Submitted the item)
  • -email is unavailable- added by hardy (cc people who care about the image package)
  • -email is unavailable- added by hardy (cc people who care about the image package)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-05 avinoam StatusNone Done
        Open/ClosedOpen Closed
    2017-08-02 hardy Attached File- Added houghpeaks_V5.m, #41393
    2017-08-01 hardy Attached File- Added houghpeaks_V4.m, #41385
    2017-07-17 hardy Attached File- Added houghpeaks_V3.m, #41218
    2017-07-12 hardy Attached File- Added houghpeaks_V2.m, #41194
    2017-07-11 hardy Attached File- Added houghpeaks_V1.m, #41178
        Carbon-Copy- Added avinoam
        Carbon-Copy- Added carandraug

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code