Make roofer deterministic: seed, address-independent orders, uninitialised reads

[guhur]

Addresses #138.

Why

Running the same roofer build twice on the same inputs does not always give the same models. We hit this while comparing roofer versions and settings at scale: differences between two variants were drowned in run-to-run noise.

On 7,393 buildings (a French municipality plus a national sample of houses, IGN LiDAR HD), current main gives:

	buildings whose model differs between two identical runs
one roofer process per building, two runs	79
same, but with a shorter output path	217
tile mode, one process, -j 16, 5,364 buildings, two runs	159

The output path matters because a path over 15 characters no longer fits in std::string's inline buffer. It is then heap-allocated, which shifts every later allocation.

Causes, one commit each

None of these is TBB: the Conan build does not link it.

1. Unseeded shuffle: RegionGrower_DS_CGAL::get_seeds shuffled with std::random_device. It now uses a fixed seed (the same 31415 as gridthinPointcloud).

2. Containers ordered by memory address:

   • The line regulariser kept its clusters in std::set<std::shared_ptr<…>>, that is, ordered by address. That order decided the distance heap's ties, the merge direction and the cluster ids. Clusters now carry a creation id, and the sets are ordered by it.
   • The step-edge merge in ArrangementBase iterated a hash map of face pairs. It now visits them in the order the arrangement's edges first meet them.

3. Uninitialised extremes in calc_segment: when a distance cluster holds only intersection lines, there is no other line to bound them. calc_segment then received an empty list and returned its uninitialised pmin/pmax. A lone intersection line is now bounded by itself, and calc_segment starts from the centroid.

4. Snapper order: CGAL's edge iterator reports each edge from whichever of its two faces has the lower address. As a result, "the first short edge" and the order in which the snapped arrangement was rebuilt both depended on the heap. The snapper now:

   • collapses the shortest short edge, with ties broken by its endpoints' coordinates;
   • rebuilds the arrangement in vertex-index order;
   • seeds the dangling-constraint peeling in vertex order instead of hash-map order.

   The new CDT labelling already breaks label ties by source face id, so it needed no change.

5. Cropper: in PointsInPolygonsCollector::do_post_process, the loop that merges the buffer ground points declared size_t poly_i; without initialising it.

6. Thinning: gridthinPointcloud skipped cells with i > size(), so a point mapping to i == size() read one past the count raster. Whether that point was kept, and therefore every later random draw, depended on that memory. Buffer ground points outside the footprint's raster hit this case.

I found the causes by fingerprinting each stage's output (planes, alpha rings, lines, regularised lines, arrangement, snapper, input ground points), then comparing two diverging runs of the same building.

Verification

Same 7,393 buildings and inputs, comparing geometry and all rf_* attributes except rf_t_run:

	main	this PR
per building, two runs	79 differ	0
per building, short vs long output path	217 differ	0
tile mode -j 16, two runs (5,364 buildings)	159 differ	0

Effect on the models compared with main:

• 348 of 6,777 buildings with an LoD 2.2 model change;
• the median RMSE change is 0;
• 10 change volume by more than 1 %. The largest two are buildings where main itself flips between two models from run to run, depending on the heap layout. This PR picks one of them every time.

The six changes are independent. I can split them into separate PRs if that is easier to review.

Noticed, not changed here

gridthinPointcloud and RasterisePointcloud compute a cell index for points outside the footprint's bounding box with static_cast<size_t>(floor(...)). Negative values wrap. Points beyond the right edge land in the next row. So buffer ground points are either dropped by the thinning or counted against an unrelated cell. This PR only removes the out-of-bounds read, because fixing the mapping changes which ground points are kept.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LZBFfd55Us1zg7chCUDs5C

[Ylannl]

Thanks, this is a welcome contribution. I'll review it as soon as I have time.