bugGNU Octave - Bugs: bug #54874, [octave forge] (image) hough...

 
 

bug #54874: [octave forge] (image) hough transform size is not compatible with Matlab

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Mon 22 Oct 2018 04:12:06 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 01 Nov 2018 05:35:07 AM UTC, comment #5: 
Avinoam Kalma <avinoam>
Group Member
Wed 31 Oct 2018 10:14:36 PM UTC, comment #4: 

I would like to fix now the size issue, and add the missing "RhoResolution" later.
I will open a new bug report on this.
Thanks

Avinoam Kalma <avinoam>
Group Member
Wed 24 Oct 2018 05:23:31 PM UTC, comment #3: 

I think the current functionality of hough.m (and the underlying C file) is tested deep enough by the existing test cases in hough.m . So no, I don't have any additional tests for this purpose.

But since you are currently "into" the C code for the rho values: Do you have time (and enough insight) to ADD the missing "RhoResolution" parameter to our functions? If you start doing this, then please let me know. I could then provide additional (Matlab compatible) test cases for this new functionality.

Hartmut <hardy>
Mon 22 Oct 2018 07:53:48 PM UTC, comment #2: 

Hi,

All the tests pass, and the previous xtest are compatible with Matlab. If you have more tests to add, you are welcome.
All my changes are pushed to the default branch.

Avinoam Kalma <avinoam>
Group Member
Mon 22 Oct 2018 07:33:07 PM UTC, comment #1: 
  • Do all the currently present tests (and maybe even some of the previously failing xtests) still pass after your suggested change?
  • This is a "compatibility breaking" change for the long standing hough_line.cc in Octave. So I assume this change needs to go into the default branch?


Hartmut <hardy>
Mon 22 Oct 2018 04:12:06 AM UTC, original submission:  

Matlab:


>> size(hough (zeros(10)))

ans =

    27   180

>> size(hough (zeros(20)))

ans =

    55   180


Octave:


>> size(hough (zeros(10)))

ans =

    29   180

>> size(hough (zeros(20)))

ans =

    57   180


According to Matlab documentation (https://www.mathworks.com/help/images/ref/hough.html)
The Hough transform matrix, H, is nrho-by-ntheta where:


nrho = 2*(ceil(D/RhoResolution)) + 1, and
D = sqrt((numRowsInBW - 1)^2 + (numColsInBW - 1)^2).


While hough_line.cc set the size as:


Matrix size (1, 2);
size (0) = r; size (1) = c;
const double diag_length = sqrt (size.sumsq ()(0));
const int nr_bins = 2 * (int)ceil (diag_length) - 1;


Which should be changes to


const double diag_length = sqrt ((c-1)*(c-1)+(r-1)*(r-1));
const int nr_bins = 2 * (int)ceil (diag_length) + 1;


This change makes hough tests compatible with matlab


Avinoam Kalma <avinoam>
Group Member

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    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 group members can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-01 avinoam StatusNone Fixed
        Open/ClosedOpen Closed
    2018-10-22 avinoam Carbon-Copy- Added hardy
        Carbon-Copy- Added carandraug

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code