Fix over-rounding, 3.5x perf, merge_holes option, fold repairs, quality test suites - #6
Merged
Merged
Conversation
Simplify strips all collinear vertices from straight edges, leaving arbitrarily long segments. Chaikin's corner cuts scale with segment length, so shapes with long straight edges were massively over-rounded (a large square with a small segment_length collapsed into a circle). Re-segmentize after every simplify step so Chaikin never sees segments longer than 4 * segment_length: right-angle corners now round by ~segment_length (the deviation budget simplify already allows) while shallow facet chains from simplified curves still blend smoothly. Covers all paths through _smoothify_geometry (polygon variants, linestrings, and the dissolved geometry before the final pass). Add regression tests pinning corner rounding to segment_length scale for polygons and linestrings, plus a guard that staircase artifacts still get smoothed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Profile-driven optimizations on examples/Water.gpkg (6.8s -> 1.9s), each verified to keep output differences sub-pixel (within the algorithm's own start-point noise floor): - Drop the reversed-direction smoothing variants: Chaikin corner cutting is direction-invariant on closed rings, so they duplicated the forward variants bit-for-bit and only inflated the variant union (output unchanged) - Subtract holes in one difference call against their union instead of an intersection + difference pair per hole (output unchanged) - Use mitre joins for the tiny merge/dissolve buffers: round joins added ~8 arc vertices per corner (9x vertex inflation) at millimetre scale - Cap pre-union Chaikin at 2 iterations; finer detail was erased by the post-union simplify anyway, while doubling union vertex counts - Area-preservation root finder: one-sided bracketing from the linear estimate, cached evaluations, and a step tolerance derived from the area tolerance via the perimeter (roughly halves buffer calls) - Smooth congruent geometries (translated copies, common in raster data) once and translate the result to each occurrence; keys are 16-byte BLAKE2b digests so key memory is independent of geometry size, and 3D geometries are excluded so Z values are never swapped or dropped Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tput Two artifact classes produced fold-like defects on shapes with features about one segment_length wide (where the simplify tolerance equals the feature width and the start-point variants disagree): - Variant-union folds: the union of disagreeing variants carries a forked slit that the area-preservation shrink sharpens into a cusp (~150 degrees on a real-world case; user-reported on a 1x2-pixel rectangle). The final result is now checked for sharp concave turns — legitimate thin features only have sharp convex hairpins — and on detection the final pass is redone from the variant union sealed with a small closing (dilate-erode at segment_length / 4). - Clipped-hole cusps: a smoothed hole can cross the independently smoothed exterior; the subtraction then clips it, leaving tangential cusps (up to 180 degrees, pre-existing). Closing cannot seal tangential wedges, so these are repaired with an opening + closing, which removes hair-thin material needles and seals slits. Both repairs are gated on detection (~1% of real-world features), bounded by their segment_length / 4 radius, and leave area preservation within tolerance. Regression tests cover the reported geometry at its real UTM coordinates through the GeoDataFrame pipeline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Holes from diagonally adjacent raster cells touch at a corner; smoothed independently they become separate rounded openings that overlap or leave a fake land bridge. merge_holes (default True, mirroring the other merge flags) joins holes that touch or nearly touch so they smooth into one coherent opening. Pass merge_holes=False for the previous behaviour. Details: - The join epsilon is segment_length / 250 — larger than the shell merge epsilon because the GeoDataFrame pre-buffer shrinks holes and pulls corner-touching pairs apart by twice segment_length / 1000 - Holes that do not actually merge keep their original ring, so the epsilon never leaks into their area preservation - Composes with merge_collection: a hole split across feature boundaries is reunited by the dissolve and then merged with any hole it touches; overlapping shells resolve holes by union semantics first - Worked examples in examples/merge_holes_examples.ipynb Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tests/test_water_quality_sweep.py: runs examples/Water.gpkg through 14 settings combinations (single knobs plus interactions) and screens every output for invalid/empty geometries, sharp concave folds, area drift, and dropped features; also pins the parallel path to bitwise equality with serial output. Marked slow. The sweep's first calibration run is what surfaced the pre-existing clipped-hole cusp. - tests/test_congruence_dedup.py: translated copies must get exactly translated results, distinct shapes must not merge, and 3D geometries keep their Z values (regression for the dedup Z-corruption bug) - tests/test_synthetic_blob_fuzz.py: seeded random raster-style blobs screened with the same defect checks, complementing the real-data sweep - benchmarks/bench_water.py: single-core timing/profiling benchmark with baseline output comparison (baseline file gitignored) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the merge_holes parameter to the parameters table, link the new merge_holes example notebook, extend the pipeline description with hole joining and the fold-repair pass, note the congruent-shape dedup under performance considerations, and fix a 'shapley' typo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
images/generate_pipeline_graphic.py runs a rasterized demo blob through a simplified version of the real pipeline (using the library's own building blocks) and renders each intermediate product: pixelated input, densify, rotated+simplified variants, per-variant Chaikin, variant union, and the final smooth with area restoration. The resulting six-panel figure is embedded in the README's How It Works section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two of the four rotated variants simplified to identical polygons on the previous demo blob and overprinted each other, so the figure appeared to show only two rotations. Reposition the blob lobes so all variant pairs differ visibly (min pairwise symmetric difference 14.4 vs 0.0), and give each variant a distinct linestyle so coincident stretches remain individually traceable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Anything from about half the raster pixel size and up produces reasonable output; larger values round more, smaller values stay more faithful to the input geometry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Refresh all example notebook outputs and examples/Water_Smoothed.gpkg with the current pipeline (perf work, merge_holes, fold repairs), and add a spell-check word to the workspace settings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wrignj08
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch grew from the over-rounding fix into the full 0.2.4 release content.
Fixes
segment_length=10collapsed into a circle. Simplify strips collinear vertices, so Chaikin's cuts scaled with edge length; geometries are now re-segmentized after every simplify so corner rounding stays at ~segment_length(deviation 14.9% of area → 0.02% on the square case).segment_lengthwide (user-reported heart-shaped cusp on a 1×2-pixel rectangle): start-point variants disagree about such features and the area-preservation shrink sharpens the resulting slit into a cusp. The final result is screened for sharp concave turns and recomputed from a sealed variant union when one is found.segment_length / 4.Performance
~3.5x faster single-core on
examples/Water.gpkg(6.8s → 1.9s), output differences sub-pixel, area preservation unchanged or better:Features
merge_holesoption (defaultTrue): touching holes (e.g. diagonally adjacent raster cells) smooth into one coherent opening; composes withmerge_collectionacross feature boundaries. Worked examples inexamples/merge_holes_examples.ipynb.Tests & docs
merge_holesdocs, clarifiedsegment_lengthguidancebenchmarks/bench_water.pyfor future perf workFull breakdown in CHANGELOG.md (Unreleased).
🤖 Generated with Claude Code