patchGNU Octave - Patches: patch #9602, [octave forge] (image) multithresh

 
 

patch #9602: [octave forge] (image) multithresh

Submitter:  Ricardo Fantin da Costa <ricardofantin>
Submitted:  Fri 16 Mar 2018 03:39:54 PM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

  Spam posted by anonymous
Tue 13 Nov 2018 07:59:00 PM UTC, comment #2: 

I have reviewed this new code contribution. Thank you for providing it to Octave!

There are still a couple of issues with this new function. (I consider at least the first two of those issues to be major):

  • The code execution is very SLOW. For example a 256-by-256 pixel grayscale image takes 3s to compute for 2 thresholds and 300s to compute for 3 thresholds. One idea to fix this would be to use already existing optimization routines in Octave (e.g. fminsearch) instead of programming the search algorithms "from scratch".
  • The results are NOT Matlab COMPATIBLE.
    • Even for just 1 single threshold, the output is not compatible to the result of the (already existing, Matlab compatible) Octave function graythresh.m (I have tested this on cameraman.tif)
    • Also for 2 and 3 thresholds (used on cameraman.tif) the outputs are not compatible.
  • Could you quote a (literature) source of your algorithm? Have you used the referenced paper cited on the Matlab help page?
  • The behavior on RGB images (and all images of dimension that are not 2d) are not Matlab compatible. Matlab claims to flatten all image pixels to 2d before processing. But this code uses im2gray instead.
  • The warning message (code line 68) should use the filename in front: "multithresh.m: Warning ...".
  • The help string needs some spell-checking.
  • The help string should mention the possible return value "quality".
  • This functions also accepts histograms instead of images (Matlab does not claim to be able to do this.) This might well be fine (and a nice Octave-only feature) if the rest of the function behaves compatible.
  • Matlab accepts some more input types, not just uint8 and uint16.
  • The tests leave a plot window open. It might be a good idea to split this test in a pure test (with no plot windows) and a demo block (with plot windows.)
  • There is some unnecessary double calculation of the non-zero values, code line 60 and 61.
  • Other use-cases should also get tests. E.g.
    • input validation
    • several number of requested thresholds, n=1, n=2, n=3 at least
    • compatibility of some results to Matlab
    • RGB input images
    • A Nd image, maybe with 4 dimensions


Conclusions: In my opinion this function's code would need some major rework to be included into the image package. But the core functionality seems to be mostly there. And this would also be a nice function to have in Octave's image package.

@Ricardo: So if you can spend the extra effort to tackle the above mentioned issues, then please go ahead. I would be happy to have a second look at your next version of this function.

Hartmut <hardy>
Wed 07 Nov 2018 05:52:28 AM UTC, comment #1: 

Pleae review

Avinoam Kalma <avinoam>
Group Member
Fri 16 Mar 2018 03:39:54 PM UTC, original submission:  

Hello,
I finished the multithresh function.

It is my first code contribution to Octave. Any feedback is welcome.

I tested in two images and the result was the same of MATLAB.


Ricardo Fantin da Costa <ricardofantin>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43572:  multithresh.m added by ricardofantin (5KiB - 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 hardy (Posted a comment)
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by ricardofantin (Submitted the item)
  •  

    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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-11-07 mtmiller Carbon-CopyRemoved 80942 -
    2018-11-07 avinoam Carbon-Copy- Added hardy
        Carbon-Copy- Added carandraug
    2018-03-31 mtmiller Summarymultithresh () for the image package [octave forge] (image) multithresh
    2018-03-16 ricardofantin Attached File- Added multithresh.m, #43572

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code