[GRASS-dev] i.superpixels.slic ( was: #3142 Implementing SLIC image segmentation)

Markus Metz markus.metz.giswork at gmail.com
Thu Jan 26 06:18:34 PST 2017


On Mon, Jan 23, 2017 at 11:19 AM, Rashad Kanavath <
mohammedrashadkm at gmail.com> wrote:
>
>
>
> On Mon, Jan 23, 2017 at 9:22 AM, Moritz Lennert <
mlennert at club.worldonline.be> wrote:
>>
>> Hi Rashad,
>>
>> On 22/01/17 11:00, Rashad Kanavath wrote:
>>>
>>> I fixed all the points from last mail.
>>
>>
>> Wonderful !
>>
>> It seems that you missed one point (see in the ticket #3142/#3247).
>>
>> Thank you so much for this great job !
>
>
> Thanks. Sorry that I missed it. I will push a fix for both and update the
tracker too.

I found time to review the code and discovered a few more issues:

L210
  pdata  = G_malloc (sizeof (int) * group_ref.nfiles);
must be
  pdata  = G_malloc (sizeof (int *) * group_ref.nfiles);

L232
    pdata[nf][index] = ((int *) i_band)[col];
works only if the input is CELL_TYPE
i_band should be *DCELL, pdata should be ***DCELL such that L232 becomes
    pdata[row][col][nf] = i_band[col];
the remaining code needs to be updated for changes in pdata

L468
no need to allocate and free a new row buffer for each output row

in SLIC_EnforceLabelConnectivity()
allocating sizeof(int) * nrows * ncols) for both xvec and yvec can cause
OOM errors. Superpixels should be much smaller, thus dynamic reallocation
would be better.

L586
this is the wrong place to update numlabels, better do
numlabels = SLIC_EnforceLabelConnectivity()
in main() L457

NULL handling must be implemented
cells where any input band is NULL must be skipped.
Initializing seeds must be changed at L323-327 to avoid initializazion with
NULLs

A few variables are allocated as vectors even though they hold some info
about each cell. They should be allocated as matrices. E.g. klabels is
currently accessed as klabels[row * ncols + col], better would be
klabels[row][col]. Equivalent for pdata and distvec. This change will make
it easier to use the segment library for managing temporary data.

The spatial and spectral distances are currently not comparable, the rather
cryptic compactness parameter needs to be used to make them comparable and
determine the weight for the spatial and spectral distances. It would be
easier to understand if the spatial and spectral distances are separately
scaled such that each is in the range [0, 1]. This is already done in
i.segment and also described in the SLIC paper in formula 2. Compactness
could then be changed to be the weight for spatial distance with default
0.5. Values > 0.5 would favour the spatial distance and cause more compact
superpixels while values < 0.5 would favour spectral distance and cause
less compact but spectrally more homogeneous superpixels.

The option to perturb seeds could be useful to get more robust initial
seeds. A similar mechanism could also be used to find a non-NULL cell as
initial seed location.

Currently the module has an option to specify the desired number of
superpixels. That makes comparison of the results for the same input but
different computational regions difficult. Instead of the number of
superpixels, the average superpixel size could be an input option, that
would make results comparable.

Markus M

>
>
>>
>> Moritz
>>
>>
>>
>>>
>>> On Fri, Jan 6, 2017 at 9:28 AM, Markus Metz
>>> <markus.metz.giswork at gmail.com <mailto:markus.metz.giswork at gmail.com>>
>>> wrote:
>>>
>>>
>>>
>>>     On Tue, Dec 27, 2016 at 9:05 PM, Rashad Kanavath
>>>     <mohammedrashadkm at gmail.com <mailto:mohammedrashadkm at gmail.com>>
wrote:
>>>
>>>
>>>
>>>         I had pushed a updated version of code:
>>>
>>>         Done:
>>>         1. input is a imagery group from i.group (handles all raster in
>>>         the group)
>>>         2. removed rgb2lab conversion
>>>         3. implementation of id output map
>>>         4. replaced  fmin with MIN and MAX macro
>>>         5. compactness is a new option to module? (default is 20 from
paper)
>>>
>>>     Wonderful, thanks!
>>>
>>>
>>>         TODO:
>>>         1. add option for SLICO
>>>         2. use segment library to avoid memory limitation when reading
>>>         raster data
>>>
>>>         Hope this works now. Let me know If I missed something else.
>>>
>>>     Code clean-up:
>>>     - please use tools/grass_indent.sh
>>>     - move variable declarations to the beginning of a code block
>>>     - convert C++ style comments to C style comments
>>>     - line 155
>>>       int sz = nrows * ncols;
>>>
>>>     can cause integer overflow. Use off_t (large raster maps can only be
>>>     handled with LFS, i.e. off_t being a 64bit integer).
>>>
>>>     - fix L157: must be
>>>       int **pdata;
>>>
>>>       pdata = G_malloc(sizeof(int) * group_ref.nfiles);
>>>
>>>     - Lines 317,318 are correct, line 320 is wrong
>>>
>>>
>>>         And...
>>>
>>>         Happy New Year to all :-)
>>>
>>>     Happy new year to you too!
>>>
>>>     Markus M
>>>
>>>
>>>
>>>         On Thu, Dec 22, 2016 at 10:12 AM, Markus Neteler
>>>         <neteler at osgeo.org <mailto:neteler at osgeo.org>> wrote:
>>>
>>>             On Thu, Dec 22, 2016 at 12:01 AM, Markus Metz
>>>             <markus.metz.giswork at gmail.com
>>>             <mailto:markus.metz.giswork at gmail.com>> wrote:
>>>             ...
>>>             > Adding
>>>             >
>>>             > #undef MIN
>>>             > #define MIN(a,b) ((a) < (b) ? (a) : (b))
>>>             >
>>>             > as in other GRASS modules is a nice solution to keep
things simple and
>>>             > portable.
>>>
>>>             ... how about a adding this and similar definitions to
gis.h?
>>>
>>>             markusN
>>>             _______________________________________________
>>>             grass-dev mailing list
>>>             grass-dev at lists.osgeo.org <mailto:grass-dev at lists.osgeo.org>
>>>             http://lists.osgeo.org/mailman/listinfo/grass-dev
>>>             <http://lists.osgeo.org/mailman/listinfo/grass-dev>
>>>
>>>
>>>
>>>
>>>         --
>>>         Regards,
>>>            Rashad
>>>
>>>
>>>
>>>
>>>
>>> --
>>> Regards,
>>>    Rashad
>>
>>
>
>
>
> --
> Regards,
>    Rashad
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20170126/c7855210/attachment-0001.html>


More information about the grass-dev mailing list