#1248: r.thin may be broken
-------------------------+--------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.thin | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------
I recently tried to use r.thin in GRASS 7 (compiled from trunk a few days
ago) and it seems to be behaving oddly. So I tested it against the
Spearfish demo set. I ran r.watershed to generate a stream network
(delete_streams) and then tried to thin it. The network generated by
r.watershed is already down to a single cell in most places; only a few
need thinning. But the r.thin output seems odd. It says it is deleting
thousands of cells each iteration, with the number of cells deleted
dropping by 8 each iteration. After 200 iterations, r.thin says that
thinning is not completed. Looking at results, only a small fraction of
the stream network was actually generated by r.thin. Here is an example of
r.thin output:
r.thin input=delete_streams@PERMANENT output=delete_streams_thinned
File delete_streams@PERMANENT -- 466 rows X 633 columns
Bounding box: l = 2, r = 634, t = 2, b = 467
Pass number 1
Deleted 2194 pixels
Pass number 2
Deleted 2186 pixels
Pass number 3
Deleted 2178 pixels
Pass number 4
Deleted 2170 pixels
Pass number 5
...
Deleted 618 pixels
Pass number 199
Deleted 610 pixels
Pass number 200
Deleted 602 pixels
Thinning not completed, consider to increase 'iterations' parameter.
Output file 466 rows X 633 columns
Window 466 rows X 633 columns
#1248: r.thin may be broken
-------------------------+--------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Raster | Version: svn-trunk
Keywords: r.thin | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------
Comment(by mmetz):
Replying to [comment:2 cmbarton]:
> I just tested this in GRASS 6.4.1 and r.thin works fine. All thinning of
the same map in spearfish was completed in 1 pass.
Try r44645. The bug is that G_get_map_row() in 6.x converts NULLs to
zeros, but there is no equivalent to G_get_map_row() in 7.0, where
G_get_map_row() is usually replaced with Rast_get_c_row() which does not
convert NULLs to zeros. The current fix converts NULLs to zeros in
r.thin/io.c, but a proper fix would be to update r.thin and test against
NULL values with !Rast_is_c_null_value() instead of e.g. buf[col] != 0.
Padding should also use NULL and not zero. Therefore I would opt to change
the status to enhancement if the current fix is working.
And all modules that use G_get_map_row() in 6.x should be checked...
Replying to [comment:7 cmbarton]:
> Replying to [comment:6 hamish]:
>
> Markus M suggested keeping this open, at least as an enhancement,
because he didn't feel that this was a proper fix.
>
It should be properly fixed in r44866. Zero is now a valid number, all
non-NULL cells are treated as lines.
Michael
____________________
C. Michael Barton
Director, Center for Social Dynamics & Complexity
Professor of Anthropology, School of Human Evolution & Social Change
Arizona State University