[Octave-patch-tracker] [patch #9602] [octave forge] (image) multithresh

[Hartmut]

Follow-up Comment #2, patch #9602 (project octave):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?9602>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/