Repository navigation
Increment 35: the land-cover clean-up step made faster, output byte-identical - #229
Merged
Merged
Conversation
…gen and Lagan, for increment 35's design (@Perf) One --stats run and one cProfile run per case on battery; the step is 6.9 s (67 % of the run) on Numedalslagen and 7.8 s (38 %) on Lagan. Largest sub-steps: the outline rule (2.8 s / 2.5 s, mostly GEOS union_all and make_valid), the two clips (1.4 s / 1.2 s), coverage_clean (1.2 s / 0.9 s); on Lagan the per-line covers and intersection in _add (1.5 s) and, before the step, the pyproj reprojection of every polygon (4.1 s). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… its measured probes (@architect) Four exact Python changes, about 20 net lines, every output byte-identical: the cover polygons reprojected in one call split over up to 8 threads; _untouched widened to lines whose ends touch the outline (DE-9IM T**F**FF*); covers skipped where it holds; one outline STRtree per clean-up. Measured on monkeypatched runs: Lagan features clip 11.74 to 6.74 s, Numedalslagen 6.29 to 6.00 s, meshes and features: lines identical. Skipping the region and domain clips left out: GEOS's overlay rewrites ring order (ring_order.py, stage_check.py). ROADMAP row 35 numbered and set to designed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd one line number, by the main session Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…failing tests (@tester) docs/increments/35-cleanup-speed.md section 5, tests 1 to 6, Python only. 15 tests fail at this commit, 275 others in the two files pass: .venv/bin/python -m pytest tests/python/test_crs.py tests/python/test_feature_input.py -q --no-cov tests/python/test_crs.py, TestChunkedReprojector (test 1, invariant-critical): - 10, 500 000 (= 2 * CHUNK_POINTS), 500 001 and 1.2 million points drawn uniformly in EPSG:3035 over southern Sweden, to EPSG:3006, np.array_equal against one Transformer.transform call (kills M4: a point dropped or doubled changes the shape or a value). Red where crs.CHUNK_POINTS is read; the 10- and 1.2-million-point rows pass today (one call is its own oracle). - CHUNK_POINTS == 250_000 and WORKERS == min(8, os.cpu_count() or 1). tests/python/test_feature_input.py: - UNTOUCHED_CASES (test 3, invariant-critical) gains ends-on-the-outline, one-end-on-the-outline, end-on-a-hole (the design review's case) and end-on-a-domain-vertex, all True, and ends-on-the-outline-through-a-domain-vertex, False. Measured here (GEOS 3.14.1): the pattern T**F**FF* agrees with every row, and GEOS gives the line back unchanged on every True row. test_r2_every_true_row_is_what_geos_ gives_back runs that oracle over each True row; the random test_r2 stays. Red: the four new True rows (today contains_properly refuses them). - TestOneTransformPerSource (test 2): three EPSG:3035 squares under the corine map with the CLI's clean-up defaults; fi.reprojector wrapped to count apply calls: expected [15] (one call, every cover vertex), today [5, 5, 5]. A vertex at x = 1e300 is refused with today's text naming its fid (passes). - TestClippedCount (test 4, invariant-critical): _Tally._add called directly with two chains; BaseGeometry.covers and shapely.covers both wrapped (the method does not go through the function for a scalar). One untouched plus one crossing chain: clipped 1 and one covers call (today 2); two untouched: clipped 0 and no call (today 2). Kills M3 (clipped from the flags alone). - TestOneOutlineTreePerCleanup (test 5): the CORINE extract over the quarter-circle domain, shapely.STRtree subclassed to record the feature_input.py frames under _clean; one tree not under snap_to_outline (today 8, one per _outline_lines call) and none in _outline_lines. The lines of every _outline_lines call equal a tree-free cut (full distance matrix of ring points against outline segments, then _chains): 86 lines, passes today and after. - TestTheSetIsUnchanged (test 6): the same run as built and with 30b's _untouched and the one-call reprojector (both verbatim) monkeypatched: fid, mask, every line's WKB, clipped, outside, empty, cover_vertices equal. Passes today; the guard for green. Not written: no real-catchment test. Section 5 says "No real catchment in CI" and section 7 gives the byte-identical meshes to @Perf's quick check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dentical (@developer) docs/increments/35-cleanup-speed.md section 3, Python only; the 15 tests of c413019 pass, nothing under tests/ touched. crs.py (3.1 b): CHUNK_POINTS = 250_000 and WORKERS = min(8, os.cpu_count() or 1). reprojector's apply splits an array of 2 * CHUNK_POINTS points or more with np.array_split into min(WORKERS, n // CHUNK_POINTS) chunks (uneven split kept in order) and maps them over a ThreadPoolExecutor opened and closed inside the call; each worker thread builds its own Transformer in a threading.local, the calling thread's is built eagerly as before so a bad CRS still refuses at construction. Smaller arrays take the one-call path unchanged. feature_input.py: - 3.1 (a) _take appends cover polygons unmoved; after the loop one shapely.transform over an object array of them, then _finite per polygon with today's FeatureError text naming the fid; timed into self.seconds. - 3.2 _untouched: relate_pattern(domain, line, "T**F**FF*") replaces contains_properly; the other three clauses and their order stay. - 3.3 _add evaluates _untouched once per chain and asks covers only for the chains it did not pass (all() short-circuits on the first uncovered one). - 3.4 _outline_tree(outline) -> (segments, STRtree), built once in _clean; _outline_lines(polygon, tree) takes the pair. The design wrote _outline_lines(polygon, outline, tree); the outline argument would have been unused once the tree carries its segments, so it is dropped (only caller: _clean). Verified at this head, this worktree's .venv (shapely 2.2.0 / GEOS 3.14.1, pyproj 3.8.0, _core as master): pytest -q --no-cov: 7143 passed, 17 skipped, 0 failed (559 s) mypy strict, ruff check, ruff format --check: clean check_prohibited_deps, check_detria_boundary, check_citations: OK rasputin mesh, the quick check's two catchment cases, --out X.vtk --binary, whole-file sha256 as docs/increments/35-probes/mesh_check.py hashes: numedalslagen 349d0b5632a28af6024210556a0bd0b6f35e5ba99378c18c742d4b666acfdcfe features: 22 kept (15 cut at the domain outline), 4638 outside the domain, 0 empty lagan 6c110f9eb1fb6a741bd4a19a954a91b7a7424d43f7ba04c354d25f9a42495fa0 features: 23 kept (0 cut at the domain outline), 4193 outside the domain, 0 empty both equal to the design's recorded digests (probes README, item 4) and features: lines. No C++ built; no sanitizer run (Python only). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dded for M3 (@tester) docs/increments/35-cleanup-speed.md section 5, the invariant-critical tests 1, 3 and 4: tests/python/test_crs.py::TestChunkedReprojector, tests/python/test_feature_input.py::TestUntouched and ::TestClippedCount. Each mutant planted in a scratch copy of e805d37 made by tools/scratch_copy.py in the session scratchpad (removed afterwards), the three classes run (36 tests), the file restored from `git show e805d37:<path>` and touched, the unmutated run green (36 passed) after each. This machine has 10 cores, so WORKERS = 8 and the chunked path runs; on a one-core runner M4 cannot be killed (one chunk). Kill record (mutant | where planted | killing test, fixture row | result): M1 relate pattern "T**F**FF*" -> "T*****FF*" (plain covers) src_python/tin_engine/feature_input.py, _untouched TestUntouched::test_r1_case_by_case: along-then-inside, domain-vertex-on-the-line, touching-a-hole, ends-on-the-outline-through-a-domain-vertex, closed-ring-touching-the-outline-at-its-start, closed-ring-touching-the-outline-mid-ring (mutant True, False expected); and test_r2_where_it_is_true_geos_gives_the_line_back_unchanged. along-the-outline does not kill it, as the design review corrected. KILLED (7 of 36 failed) M2 `and bool(shapely.is_simple(line))` dropped src_python/tin_engine/feature_input.py, _untouched TestUntouched::test_r1_case_by_case: self-touching-at-a-vertex, self-crossing, doubling-back; and test_r2_where_it_is_true_geos_gives_ the_line_back_unchanged. KILLED (4 of 36 failed) M3 `self.clipped += not all(u or dom.covers(g) ...)` -> `not all(whole)` src_python/tin_engine/feature_input.py, _Tally._add TestClippedCount::test_one_untouched_and_one_crossing_chain_is_one_ clipped_feature, by the covers spy only (`len(covers_calls) == 1`, got 0): the count stays 1 there, since not all([True, False]) is True either way. KILLED (1 of 36 failed), but by the spy, not the count. Over the whole test_feature_input.py the count alone is caught only by test 6 (TestTheSetIsUnchanged::test_as_built_equals_the_old_rules, the CORINE fixture), which is not invariant-critical. Added TestClippedCount::test_a_covered_chain_the_rule_refused_is_not_clipped: a chain along BOX's east side, covers True, _untouched False, GEOS gives it back whole, clipped 0 and covers asked once; the mutant counts 1. Re-planted with the new test copied in: KILLED by the count (1 == 0). M4 chunk boundary off by one in apply, two variants src_python/tin_engine/crs.py, reprojector.apply M4-dropped: np.array_split replaced by floor-division slices, the remainder dropped. TestChunkedReprojector::test_one_point_above_the_ threshold_still_equals_one_call (n = 2 * CHUNK_POINTS + 1). KILLED (1 of 36 failed) M4-doubled: the first chunk one point longer, the boundary point doubled. TestChunkedReprojector::test_chunked_equals_one_call_bit_for_bit [exactly-two-chunks], [1.2-million-points], and test_one_point_above_ the_threshold_still_equals_one_call. KILLED (3 of 36 failed) Red-step scaffolding: none found in c413019's added lines (no xfail, skip, TODO or red note); nothing to remove. Verified at this head, this worktree's .venv: pytest -q --no-cov tests/python/test_feature_input.py tests/python/test_crs.py: 291 passed; ruff check and ruff format --check on the test file: clean. tests/ only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d, with the Status line and ROADMAP row, by the main session Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…9 of base, every mesh unchanged; baseline refreshed (@Perf) docs/benchmarks/2026-10-11/cleanup-speed/README.md: head 357808b against the branch's base, master 3e6f87e, back to back in one venv (numpy 2.5.4), AC power, medians of 3 per case. Verdict ACCEPTED against section 7 of docs/increments/35-cleanup-speed.md: all 7 meshes byte-identical to the base's; Lagan features clip 11.156 -> 6.342 s (ratio 0.569, limit 0.65); Lagan total 19.296 -> 14.542 s (0.754, limit 0.85); Numedalslagen features clip 6.092 -> 6.000 s (0.985, limit 1.0); the other cases -0.7 % to +0.8 %. Tool verdict FASTER, exit 0. The first run against the stored baseline 39e3ae3 was NO BASELINE (numpy 2.5.4 vs 2.5.3); increment 32 (merged as 3e6f87e after that baseline) changed the features meshes, so the base commit was measured with --tree as docs/increments/README.md asks. docs/benchmarks/quick/baseline-ac.json is now the head record (perf-quick-check.md rule (a), FASTER at head). Hotspots: Lagan features clip 43.6 % of the run (ruled 40.7 %; was 57.8 % on the base, where the tool raised it). Numedalslagen features clip 68 % is unruled, as on every quick check so far. For Ola. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…AP row, by the main session Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…fixed, by the main session Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Increment 35 (
docs/increments/35-cleanup-speed.md), Ola's pick of 2026-10-11 ("land-cover clean-up speed-up, then GeoPackage"). 23 net production lines, Python only (count_loc.py 3e6f87eb HEAD:crs.py+14,feature_input.py+9), against the design's estimate of about 20.@perf's profile at master (docs/benchmarks/2026-10-11/cleanup-profile/): four exact changes. The cover polygons are reprojected in oneshapely.transformcall, with point arrays over 500,000 split across up to 8 threads, oneTransformereach;_untouchedwidened to lines whose ends touch the outline (relate_pattern(domain, line, "T**F**FF*"));coversskipped where_untouchedheld; one outline STRtree per clean-up.features:lines identical.@perf's acceptance (docs/benchmarks/2026-10-11/cleanup-speed/): Lagan features clip 11.16 → 6.34 s (0.569 of base, limit 0.65), run 19.3 → 14.5 s; Numedalslågen 6.09 → 6.00 s, its clean-up stays near 6 s (GEOS overlay time, Ola's default); other cases within -0.7 to +0.8 %. Quick-check baseline refreshed.7621a99eand section 11 of the increment file).🤖 Generated with Claude Code