[GRASS-dev] [GRASS GIS] #3545: i.superpixels.slic: behaviour of step parameter confusing

#3545: i.superpixels.slic: behaviour of step parameter confusing
-------------------------------------+-------------------------
Reporter: mlennert | Owner: grass-dev@…
     Type: defect | Status: new
Priority: minor | Milestone:
Component: Addons | Version: unspecified
Keywords: i.superpixels.slic step | CPU: Unspecified
Platform: Unspecified |
-------------------------------------+-------------------------
The behavior of the step parameter in i.superpixels.slic is a bit
confusing:

{{{
GRASS 7.5.svn (Belgique31370):/data/home/mlennert > i.superpixels.slic
orthos2016_p46_internal step=200 out=test200 perturb=10 compact=1 --o

WARNING: Initialized 9 of 24 seeds

GRASS 7.5.svn (Belgique31370):/data/home/mlennert > i.superpixels.slic
orthos2016_p46_internal step=20 out=test20 perturb=10 compact=1 --o

WARNING: Initialized 1169 of 2925 seeds

GRASS 7.5.svn (Belgique31370):/data/home/mlennert > i.superpixels.slic
orthos2016_p46_internal step=2 out=test2 perturb=10 compact=1 --o

WARNING: Initialized 76 of 187 seeds
}}}

Normally, one would expect a larger number of seeds with step=2 than
step=20. This behavior comes from lines 277ff of main.c, which read:

{{{
superpixelsize = step * step;
if (step < 5) {
     superpixelsize = 0.5 + (double)nrows * ncols / n_super_pixels;

     step = sqrt((double)superpixelsize) + 0.5;
}

Why this limit at 5 ? If it is really necessary, it should at least be
documented in the man page to avoid confusion.

}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mmetz):

Replying to [ticket:3545 mlennert]:
> The behavior of the step parameter in i.superpixels.slic is a bit
confusing:
>
> [...]
>
> Normally, one would expect a larger number of seeds with step=2 than
step=20. This behavior comes from lines 277ff of main.c, which read:
>
>
> {{{
> superpixelsize = step * step;
> if (step < 5) {
> superpixelsize = 0.5 + (double)nrows * ncols / n_super_pixels;
>
> step = sqrt((double)superpixelsize) + 0.5;
> }
>
> Why this limit at 5 ? If it is really necessary, it should at least be
documented in the man page to avoid confusion.
>
> }}}
>
The reason is that with small step sizes, you would get mini-superpixels,
mostly squares. The SLIC algorithm likes a somewhat larger distance
between superpixel centers in order to produce reasonable results. You can
modify the condition to `step < 2` and see what happens.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mlennert):

Replying to [comment:1 mmetz]:
> Replying to [ticket:3545 mlennert]:
> > The behavior of the step parameter in i.superpixels.slic is a bit
confusing:
> >
> > [...]
> >
> > Normally, one would expect a larger number of seeds with step=2 than
step=20. This behavior comes from lines 277ff of main.c, which read:
> >
> >
> > {{{
> > superpixelsize = step * step;
> > if (step < 5) {
> > superpixelsize = 0.5 + (double)nrows * ncols / n_super_pixels;
> >
> > step = sqrt((double)superpixelsize) + 0.5;
> > }
> >
> > Why this limit at 5 ? If it is really necessary, it should at least be
documented in the man page to avoid confusion.
> >
> > }}}
> >
> The reason is that with small step sizes, you would get mini-
superpixels, mostly squares. The SLIC algorithm likes a somewhat larger
distance between superpixel centers in order to produce reasonable
results. You can modify the condition to `step < 2` and see what happens.

Doing this and asking for a step of 2 both on my VISNIR aerial photos and
on the BW NC orthophoto, I get tiny superpixels of divers shapes. I don't
find them very useful for my particular application, but this should be up
to the user to decide, not imposed by the module.

If there are other reasons why step has to be above a certain threshold I
would prefer the code to just raise the value to that threshold and tell
the user about it, rather than creating a step value which is not very
understandable for users. Something like this:

{{{
if (step < 5) {
     G_warning("Setting step to the minimum value of 5.");
     step = 5;
}
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mmetz):

In [changeset:"72634" 72634]:
{{{
#!CommitTicketReference repository="" revision="72634"
i.superpixels.slic: allow step > 1 (see #3545)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mmetz):

Replying to [comment:2 mlennert]:
> Replying to [comment:1 mmetz]:
> > Replying to [ticket:3545 mlennert]:
> > > The behavior of the step parameter in i.superpixels.slic is a bit
confusing:
> > >
> > > [...]
> > >
> > > Normally, one would expect a larger number of seeds with step=2 than
step=20. This behavior comes from lines 277ff of main.c, which read:
> > >
> > >
> > > {{{
> > > superpixelsize = step * step;
> > > if (step < 5) {
> > > superpixelsize = 0.5 + (double)nrows * ncols / n_super_pixels;
> > >
> > > step = sqrt((double)superpixelsize) + 0.5;
> > > }
> > >
> > > Why this limit at 5 ? If it is really necessary, it should at least
be documented in the man page to avoid confusion.
> > >
> > > }}}
> > >
> > The reason is that with small step sizes, you would get mini-
superpixels, mostly squares. The SLIC algorithm likes a somewhat larger
distance between superpixel centers in order to produce reasonable
results. You can modify the condition to `step < 2` and see what happens.
>
> Doing this and asking for a step of 2 both on my VISNIR aerial photos
and on the BW NC orthophoto, I get tiny superpixels of divers shapes. I
don't find them very useful for my particular application, but this should
be up to the user to decide, not imposed by the module.

OK, I changed the condition to `step < 2` and updated the option
description in r72634.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:4&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mlennert):

Replying to [comment:4 mmetz]:
> Replying to [comment:2 mlennert]:
> > Replying to [comment:1 mmetz]:
> > > Replying to [ticket:3545 mlennert]:
> > > > The behavior of the step parameter in i.superpixels.slic is a bit
confusing:
> > > >
> > > > [...]
> > > >
> > > > Normally, one would expect a larger number of seeds with step=2
than step=20. This behavior comes from lines 277ff of main.c, which read:
> > > >
> > > >
> > > > {{{
> > > > superpixelsize = step * step;
> > > > if (step < 5) {
> > > > superpixelsize = 0.5 + (double)nrows * ncols / n_super_pixels;
> > > >
> > > > step = sqrt((double)superpixelsize) + 0.5;
> > > > }
> > > >
> > > > Why this limit at 5 ? If it is really necessary, it should at
least be documented in the man page to avoid confusion.
> > > >
> > > > }}}
> > > >
> > > The reason is that with small step sizes, you would get mini-
superpixels, mostly squares. The SLIC algorithm likes a somewhat larger
distance between superpixel centers in order to produce reasonable
results. You can modify the condition to `step < 2` and see what happens.
> >
> > Doing this and asking for a step of 2 both on my VISNIR aerial photos
and on the BW NC orthophoto, I get tiny superpixels of divers shapes. I
don't find them very useful for my particular application, but this should
be up to the user to decide, not imposed by the module.
>
> OK, I changed the condition to `step < 2` and updated the option
description in r72634.

Thanks, but this will lead to the same issue for step = 1. Why do you want
to make this into such a special case ? When I comment out everything
between l277 and l281, and launch the module with step=1 on the NC
orthophoto I get as many "superpixels" as original pixels. Again, I cannot
imagine a use case for this, but why should the module artificially
replace this with a calculation the origin of which is not really clear.

I would plead for respecting the user's choice of step, with a mention in
the man page that a step below 5 will lead to extremely small superpixels.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mmetz):

Replying to [comment:5 mlennert]:
> Replying to [comment:4 mmetz]:
> > Replying to [comment:2 mlennert]:
> > > Replying to [comment:1 mmetz]:
> > > > Replying to [ticket:3545 mlennert]:
> > > > > The behavior of the step parameter in i.superpixels.slic is a
bit confusing:
> > > > >
> > > > > [...]
> > > > >
> > > > > Normally, one would expect a larger number of seeds with step=2
than step=20. This behavior comes from lines 277ff of main.c, which read:
> > > > >
> > > > >
> > > > > {{{
> > > > > superpixelsize = step * step;
> > > > > if (step < 5) {
> > > > > superpixelsize = 0.5 + (double)nrows * ncols /
n_super_pixels;
> > > > >
> > > > > step = sqrt((double)superpixelsize) + 0.5;
> > > > > }
> > > > >
> > > > > Why this limit at 5 ? If it is really necessary, it should at
least be documented in the man page to avoid confusion.
> > > > >
> > > > > }}}
> > > > >
> > > > The reason is that with small step sizes, you would get mini-
superpixels, mostly squares. The SLIC algorithm likes a somewhat larger
distance between superpixel centers in order to produce reasonable
results. You can modify the condition to `step < 2` and see what happens.
> > >
> > > Doing this and asking for a step of 2 both on my VISNIR aerial
photos and on the BW NC orthophoto, I get tiny superpixels of divers
shapes. I don't find them very useful for my particular application, but
this should be up to the user to decide, not imposed by the module.
> >
> > OK, I changed the condition to `step < 2` and updated the option
description in r72634.
>
>
> Thanks, but this will lead to the same issue for step = 1. Why do you
want to make this into such a special case ?

Because ...

> When I comment out everything between l277 and l281, and launch the
module with step=1 on the NC orthophoto I get as many "superpixels" as
original pixels.

effictively a unique ID for each pixel. Regarding a superpixel as a group
of neighboring pixels with similar spectral characteristics, there are no
superpixels with step=1 and it is easier to use r.mapcalc.

> Again, I cannot imagine a use case for this, but why should the module
artificially replace this with a calculation the origin of which is not
really clear.
>
> I would plead for respecting the user's choice of step, with a mention
in the man page that a step below 5 will lead to extremely small
superpixels.

I would leave it as it is now and mention in the manual that step=1 would
not generate any superpixels, but instead a unique ID for each pixel.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------

Comment (by mlennert):

Replying to [comment:6 mmetz]:
> I would leave it as it is now and mention in the manual that step=1
would not generate any superpixels, but instead a unique ID for each
pixel.

Done in r72652. Please check if what I wrote is ok.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3545: i.superpixels.slic: behaviour of step parameter confusing
--------------------------+-------------------------------------
  Reporter: mlennert | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: minor | Milestone:
Component: Addons | Version: unspecified
Resolution: fixed | Keywords: i.superpixels.slic step
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------------------
Changes (by mlennert):

* status: new => closed
* resolution: => fixed

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3545#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>