#3287: i.zc: threshold parameter not correctly taken into account
----------------------------+-------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.2.1
Component: Imagery | Version: svn-trunk
Keywords: i.zc threshold | CPU: Unspecified
Platform: Unspecified |
----------------------------+-------------------------
i.zc is an edge detection module. In that module the parameter "threshold"
is defined as determining "the 'sensitivity' of the Gaussian filter. The
default value is 10; higher and lower values can be tested by the user.
Increasing the threshold value will result in fewer edges being found."
In the code, the value is treated as follows:
{{{
sscanf(threshold->answer, "%1lf", &Thresh);
if (Thresh <= 0.0)
G_fatal_error(_("Threshold less than or equal to zero not
allowed"));
Thresh /= 100.0;
}}}
Because of the "%1lf", only the first digit is scanned from the threshold
answer, making values 10, 100, 15, 1 lead to the same result. Also, one
cannot go below 1 as 0.5 is read as 0 and 0 is not allowed.
During tests, I found that a threshold value of 0.01 seems to give
reasonable default results.
In a quick fix, I applied the following patch to trunk.
{{{
Index: grass/trunk/imagery/i.zc/main.c
--- grass/trunk/imagery/i.zc/main.c (revision 69783)
+++ grass/trunk/imagery/i.zc/main.c (revision 70477)
@@ -87,5 +87,5 @@
threshold->multiple = NO;
threshold->description = _("Sensitivity of Gaussian filter");
- threshold->answer = "10";
+ threshold->answer = "0.01";
orientations = G_define_option();
@@ -104,8 +104,7 @@
inputfd = Rast_open_old(input_map->answer, "");
- sscanf(threshold->answer, "%1lf", &Thresh);
+ Thresh = atof(threshold->answer);
if (Thresh <= 0.0)
G_fatal_error(_("Threshold less than or equal to zero not
allowed"));
- Thresh /= 100.0;
sscanf(width->answer, "%f", &Width);
}}}
This changes the way the parameter is read to allow floating point input
and sets the default to 0.01.
However, this is a module API change as the default value has been changed
(before, it was 10/100 = 0.1) and the way to input the value has also
changed.
On the other hand, this has never worked before and no one has ever filed
a bug against this module..
So, I see the following alternatives (all include the correction to the
reading of the parameter value):
* Leave the default as is at 10 aka 0.1
* Change the default to 1, translated it internally to 0.1 (i.e. the
current default, just with a different parameter value)
* Change the default to 1, translated internally to 0.01 (i.e. what I've
experienced as a more sensible default)
* Change the default to 0.01 (as currently in trunk)
Any preferences ? Mine would probably be the third, as I find 1 as default
value more logical, and I'd rather have the module use a default value
that gives more interesting results...
--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3287>
GRASS GIS <https://grass.osgeo.org>