v.fill.holes: Add new tool to remove inner isles, keep outer boundary

[wenzeslaus]

Example

Input

Output

[wenzeslaus]

I would like for this to go to core rather than the grass-addons repo because it seems like basic functionality, but I have seen some topology issues with complex data (will post on that later).

[wenzeslaus]

What I'm missing here? I have an issue with the current implementation. If there are touching areas, it does not create a valid topology: some of the areas are not build. The same happens to be me with real-word large data and small hand-drawn data. Any comments or questions welcome.

Failing example

Input

Output

Output: boundaries and centroids

Output: vector digitizer rendering

v.build output

Number of centroids exceeds number of areas: 4 > 2
Number of incorrect boundaries: 2
Number of centroids outside area: 2 

Data

GitHub does not take the GRASS pack format even with other file extensions I tried, so it is a zipped .pack file: test2.zip

[neteler]

Does @metzm have a suggestion here on the topology issue?

[metzm]

Not needed, the operating system does it better and faster.

[wenzeslaus]

I tested the performance on a vector with 186,940 areas (180,015 after the operation). Here are its stats:

nodes=227263
boundaries=352424
centroids=166520
areas=186940
islands=61779
primitives=518944

I used perf stat for time and valgrind for the leaked memory. Here are the results:

memory release	perf stat -r 10 seconds time elapsed (3x10 runs)	valgrind LEAK SUMMARY still reachable; definitively lost
enabled	24.39, stddev 0.18 (24.181, 24.488, 24.513)	8,297,834 bytes in 145 blocks; 191,032 bytes in 184,529 blocks
disabled	23.97, stddev 0.36 (23.557, 24.212, 24.151)	248,489,855 bytes in 6,580,166 blocks blocks; 6,579 bytes in 76 blocks
difference (enabled minus disabled)	0.42 (1.7%)	-240,192,021 (regression) ; 177,950 (improvement)

I don't think the results here support the idea that OS will be "much faster" as the old doc/debugging.txt claims in a note from 2005 (OSGeo/grass-legacy@fb92c0f). The difference of averages is smaller than 2*stddev.

However, given the overall lack of memory handling and a policy to tackle that, I'm fine leaving that out for the small performance gain (for now).

[wenzeslaus]

The new version works! Thanks @metzm for the pointers!

The updated version contains documentation including images and notebook to generate them. I included some notes on topology because it was not what I expected. It as pytest-based tests.

Some features this could have, but I do not plan to implement:

• maximum (or minimum) size to remove
• keep features of other vector types (e.g. keep points)
• keep boundaries with categories (may represent lines which are at the same time boundaries)
• copy all attribute tables with layer=-1 (Now it copies only relevant rows of the attribute table for the selected layer. This usually should be the whole table because all areas with centroids are kept, so wanting to simply preserve everything makes sense.)

From my perspective, this is ready to merged. Reviews welcome.

[petrasovaa]

Perhaps change the PR name to make it clear it's a completely new tool.