bugGNU Octave - Bugs: bug #49613, image package - regionprops.m...


bug #49613: image package - regionprops.m majoraxislength broken

Submitted by:  Hartmut <hardy>
Submitted on:  Tue Nov 15 21:33:59 2016  
Category: Octave Forge PackageSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

Add a New Comment (Rich MarkupRich Markup):

You are not logged in

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


(Jump to the original submission Jump to the original submission)

Thu Apr 13 18:38:26 2017, comment #23:

I have pushed the last patch to the stable branch after splitting into two commits



I have also simplified the tests. There is no need to do:

this is enough

Carnë Draug <carandraug>
Project Member
Mon Dec 12 13:02:47 2016, comment #22:

I have tested Avinoam's latest patch (file #39195) and it looks all fine to me:

  • it resolves all mentioned issues of this bug report
    • it solves the original issue (major/minoraxislength with multiple regions), see comment #0
    • it removes the division by zero in the calculation of the ellipse orientation property, see comment #1
  • it gives Matlab compatible results on all mentioned examples in this bug report
  • it adds new tests for those improvements
    • those new tests areMatlab compatible
    • the tests all pass

If noone else finds a problem wit this patch, then I would like to see this patch commited to the image repository.

Thanks for supplying this patch, Avinoam.

Hartmut <hardy>
Sat Dec 10 21:55:19 2016, comment #21:

Thanks for checking the patch. I fixed the typo.

I hope this time it will pass the tests.

(file #39195)

Avinoam Kalma <avinoam>
Project Member
Sat Dec 10 19:20:52 2016, comment #20:

There is a small typo on your last patch, Avinoam. (It also makes one of your newly added tests fail): The number "90" from the last verbatim-box in comment #17 is mistakenly written as a "0".

I will try to run all your new tests in Matlab during the next days. If they all pass, I would be happy to see this patch applied (with the number 90 corrected).

Hartmut <hardy>
Thu Dec 8 18:43:48 2016, comment #19:

Attached a new cs, according to @Hartmut suggestios.

Please review.

(file #39177)

Avinoam Kalma <avinoam>
Project Member
Tue Dec 6 22:22:44 2016, comment #18:

Yes, I think the last "patch" in comment #17 would be fine.

Could you add some relevant tests from the discussed example images?

Hartmut <hardy>
Tue Dec 6 21:22:50 2016, comment #17:

Indeed, my patch was wrong. I tried to be compatible, but in the wrong way. It should be:

which works fine on your last example, and on your example from comment #0, except entry 12, which matlab result is 0, and mine is 90.

The same problem is with

In which Matlab result is 0, and mine is 90.

I think that the problem is that both eigenvalues are equal, so
min = max.

What about:

Avinoam Kalma <avinoam>
Project Member
Tue Dec 6 15:56:04 2016, comment #16:

With Matlab I get for the test script from comment #15:

This is different compared to the results with the patch in comment #14.

@Avinoam: Could you change your if-statement to also make those results ML compatible?

Hartmut <hardy>
Mon Dec 5 21:39:50 2016, comment #15:

I like the fact that your "patch" from comment #14 gives fully Matlab compatible results on my test image (comment #0).

But I suspect that the if-statement of this "patch" will not be Matlab compatible. We should check the Matlab result for something like this
and might need to adjust the if-statement in your "patch" accordingly.

Hartmut <hardy>
Mon Dec 5 21:12:45 2016, comment #14:

Current implementation of orientation


Changing to:

gives a Matlab compatible results. What do you think?

Avinoam Kalma <avinoam>
Project Member
Thu Dec 1 15:57:48 2016, comment #13:

Yes, the answer of Matlab R2014b is the very same as in commment #12.

Hartmut <hardy>
Thu Dec 1 07:01:28 2016, comment #12:

Thanks @Hartmut, and sorry, I did not write the correct lines:

for the commands

I get with Matlab 2010a

Do you get the same results?


Avinoam Kalma <avinoam>
Project Member
Wed Nov 30 18:26:06 2016, comment #11:

Here is the Matlab result (R2014b) from the script in comment #0 and the final line as in comment #10 :

Hartmut <hardy>
Tue Nov 29 17:00:03 2016, comment #10:

To check Matlab compatability, can some check the result of the
script from comment #0 with the line


Avinoam Kalma <avinoam>
Project Member
Sun Nov 27 19:22:12 2016, comment #9:

About the different results, I suspected it was due to
roundoff errors, so I have changed the tolerance.

Avinoam Kalma <avinoam>
Project Member
Fri Nov 25 21:34:35 2016, comment #8:

I can confirm that Avinoam's first patch (file #39060) fixes the original issue. And all tests of regionprops.m now pass on linux and on Windows (w32 tested).

@Avinoam: Do you understand why Windows gives different results (some eps only) compared to Linux for those tests you have changed?

Hartmut <hardy>
Wed Nov 23 21:52:23 2016, comment #7:

Attached a changeset for the original bug.
I also added a test for the change, and change tolerances for some
tests that failed in Windows.

Please review.

I will fix later the zero divided problem in Orientation calculation.

(file #39060)

Avinoam Kalma <avinoam>
Project Member
Thu Nov 17 18:08:00 2016, comment #6:

Sorry, the argumentation in my last post was wrong. A single pixel gets 1/12 in size, this leads to the guarantee that the majoraxislength and the minoraxislength will never become zero, I think. But this doesn't prove yet that the major_vec(1) will never be 0 in our code. Sorry.

The following image really gives me a value of major_vec(1) = 0, nevertheless the current regionprops.m DOES return the right value for the orientation:

This is due to the result of "atan(1/0)" which does give a warning (no error!), but nevertheless returns the "correct" value of pi/2 (that results in the correct orientation value of -90 degrees).

I have also tried the simple change of atan to atan2, and I do get a failing test of the Orientation feature. So a quick substitute of atan into atan2 will not do.

To conclude: I have no objection about switching from atan to atan2, but the resulting orientation values SHOULD stay the same, please. (So this would mean some additional adjustments to this code line, and double checking that the orientation results stay the same.)

Hartmut <hardy>
Thu Nov 17 17:41:51 2016, comment #5:

Regarding Avinoam's second topic (the tan function in the calculation of the ellipse orientation):

I have implemented this calculation myself (see bug #44100) two years ago. When I remember right, I succeeded to make this orientation value also Matlab compatible. So chances are high that changes in the resulting orientation values would be a regression. Do the included test cases still pass when you switch to atan2 ? (But even if yes, I would hesitate to change this).

Here is the Matlab blog article, that I used at that time to work towards Matlab compatibility of the ellipse orientation: http://blogs.mathworks.com/steve/2010/07/30/visualizing-regionprops-ellipse-measurements/

The problem with tan would only occur if major_vec(1)=0. But can this happen at all? Even a region with only one single pixel shoud receive the 1/12 value for its width (it's second moment). So I would guess it is impossible that one of the eigenvalues can become zero here. That would mean that the ellipse degrades to a 0-thin line, but there are still some pixels in it, because without any pixels it wouldn't count as a region at all... (But you could test some corner cases and try to prove me wrong, that's fine!)

Regarding Avinoam's first topic (the missing +1 for the loop counter): This sounds reasonable to fix. Thanks's for spotting this. Could you add some test for calculation the Orientation two regions at once (that have different orientations)? Maybe BW = [1 0 0 0 1; 0 1 0 1 0] or something similar?

Hartmut <hardy>
Thu Nov 17 04:36:29 2016, comment #4:

I will make two csets with tests.

Note that the tan/atan2 should be discussed, because it will
not be back compatible, since atan range is [-90:90] and atan2
range is [-180:180]

Avinoam Kalma <avinoam>
Project Member
Wed Nov 16 20:46:29 2016, comment #3:

Avinoam, could you prepare two csets (one for each issue) with your proposed fixes and a test for each of them? I will only be able to look at this next week anyway.

Carnë Draug <carandraug>
Project Member
Wed Nov 16 07:53:11 2016, comment #2:

Adding package maintainer.

Avinoam Kalma <avinoam>
Project Member
Wed Nov 16 07:51:48 2016, comment #1:

The problem is in the loop

c_idx is constant in the loop. Adding the line

fixes the problem.

Another problem is in the line:

if major_vec(1)=0 we get "division by zero" warning, and not the correct result. Maybe atan2 can fix this.

Avinoam Kalma <avinoam>
Project Member
Tue Nov 15 21:33:59 2016, original submission:

This problem happens with the current version 2.6.1 of the image package. It still worked fine with the older version of regionprops.m that is in the release image-2.4.1.

The property "majoraxislenght" and "minoraxislength" seem to be corrupted somewhere on their way through regionprops.m. If I observe this right, the results are fine when I only request those properties from a single region. But once I request those properties simultanously from several regions at once, I get corrupted results.

Here is a small script to reproduce the behavior. It uses the attached png-file "rechtecke.png":

The second plot obviously does not show the correct major and minor axis length with image-2.6.1. If you use image-2.4.1 instead you can see the correct and expected result. I will also attach those two screenshots.

Hartmut <hardy>


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

Attach File(s):

Attached Files
file #39195:  regioprops2.diff added by avinoam (6kB - application/octet-stream)
file #39177:  regioprops1.diff added by avinoam (6kB - application/octet-stream)
file #39060:  regioprops.diff added by avinoam (4kB - application/octet-stream)
file #38977:  rechtecke.png added by hardy (6kB - image/png)


Depends on the following items: None found

Items that depend on this one: None found


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

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

    Only project members can vote.


    Please enter the title of George Orwell's famous dystopian book (it's a date):



    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu Apr 13 18:38:26 2017carandraugStatusPatch Submitted=>Fixed
    Sat Dec 10 21:55:19 2016avinoamAttached File-=>Added regioprops2.diff, #39195
    Thu Dec 8 18:43:48 2016avinoamAttached File-=>Added regioprops1.diff, #39177
    Wed Nov 23 21:52:31 2016avinoamStatusNone=>Patch Submitted
    Wed Nov 23 21:52:23 2016avinoamAttached File-=>Added regioprops.diff, #39060
    Wed Nov 16 07:53:11 2016avinoamCarbon-Copy-=>Added -unavailable-
    Tue Nov 15 21:33:59 2016hardyAttached File-=>Added rechtecke.png, #38977
      Attached File-=>Added correct-result_image-2.4.1.png, #38978
      Attached File-=>Added wrong-result_image-2.6.1.png, #38979

    Back to the top

    Powered by Savane 3.1-cleanup1