Measure edge bearing over a 20 m BEARDIST chord - #30
Conversation
Per OpenLR §5.2.4 the bearing is the angle to the point BEARDIST (20 m) along the line, not the heading of a single geometry segment. On densely noded OSM curves the single-segment heading is noisy and can push a correct edge past the ±90° hard filter. bearing_at_projection now interpolates a 20 m chord from the projection point: Forward (toward the edge end) for first/intermediate LRPs, whose attributes describe the outgoing line, and Backward (from 20 m before) for the last LRP, whose attributes describe the incoming line. Both are reported in travel direction to match HERE's encoding. The chord clamps at the edge end and falls back to the other direction when degenerate. Removes the unused find_start_candidates/find_end_candidates stubs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
eb3b66e to
77368ff
Compare
|
@codex review this PR |
CI clippy runs with -D warnings; the BEARDIST direction parameter pushed find_candidates to 8 arguments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LFLFpwXmgBzigVq8staf
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77368ff4b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Point::new( | ||
| coords[i].x + t * (coords[i + 1].x - coords[i].x), | ||
| coords[i].y + t * (coords[i + 1].y - coords[i].y), | ||
| ); |
There was a problem hiding this comment.
Wrap longitudes before interpolating the bearing chord
When an edge crosses the antimeridian (for example, from longitude 179.999 to -179.999), this linear interpolation follows the roughly 360° numeric longitude difference through longitude 0 instead of the short path across ±180°. The cumulative Haversine length still treats that edge as a short segment, so the generated 20 m chord can point in the opposite direction; find_candidates may then reject the valid edge via its bearing hard filter. Interpolate on the wrapped longitude delta or use a geodesic destination/interpolation operation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2aeed9c: point_at_distance now wraps the longitude delta into ±180° and renormalizes the interpolated longitude, so the chord follows the short path across the antimeridian. Added test_bearing_chord_across_antimeridian covering both chord directions.
Addresses Codex review on #30: segments crossing the antimeridian interpolated the ~360° numeric longitude path through 0°, flipping the 20m chord's direction and potentially rejecting a valid edge via the bearing hard filter. Wrap the delta into ±180 and renormalize the result. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LFLFpwXmgBzigVq8staf
Follow-up to the Codex chord fix: the USA export has ~1,000 edges east of 170°E (Attu, Amchitka), so longitude arithmetic near ±180 matters beyond the chord interpolation. - project_point_to_segment wraps the longitude deltas and renormalizes the projected point, so candidate distances/offsets are correct for geometry near the line. - SpatialIndex::find_nearby splits a search box that pokes past ±180 into wrapped boxes, so an LRP just east of the line finds edges stored at western longitudes (and vice versa); duplicates are removed. - point_at_distance reuses the shared wrap helpers. No edge in the 2025 Q4 export crosses 180° itself (verified in BigQuery), so EdgeEnvelope's world-spanning bbox for such an edge is left as-is (conservative, never wrong). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LFLFpwXmgBzigVq8staf
…etrics Audited the rest of the repo for ±180 longitude bugs after the spatial.rs fixes: - loader.rs is safe: bearings use geodesic_bearing and its custom haversine uses sin^2 of half-angles, which is periodic in delta-lon. - tests/geo_wrap_contract.rs pins the geo-crate behaviour we rely on (HaversineClosestPoint, geodesic_bearing, haversine_distance are wrap-safe), so a dependency upgrade can't silently regress it. - qa/benchmark.py to_meters() scaled raw longitudes, which would tear a geometry crossing 180 apart and blow up Hausdorff/Frechet; longitudes are now wrapped relative to the shared centroid first. Known-and-accepted: an edge whose own geometry crosses 180 gets a world-spanning R-tree bbox (conservative; zero such edges in the 2025 Q4 USA export). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162LFLFpwXmgBzigVq8staf
Stacked on #27's branch (
worktree-valid-node-snap); merge that first.Summary
OpenLR §5.2.4 defines bearing as the angle to the point BEARDIST (20 m) along the line.
bearing_at_projectionused the heading of the single geometry segment under the projection point, which is noisy on densely noded OSM curves and could push the correct edge past the ±90° hard filter.Now interpolates a 20 m chord from the projection point:
Forward(toward the edge end) for first/intermediate LRPs — attributes describe the outgoing lineBackward(from 20 m before) for the last LRP — attributes describe the incoming lineBoth are reported in travel direction to match HERE. The chord clamps at the edge end and falls back to the other direction if degenerate.
find_candidatestakes aBearingDirection; the unusedfind_start/end_candidatesstubs are removed. Four unit tests cover kink-spanning chords, clamping, and the fallback.Benchmark (
477c043→ this branch, 47,931 codes, regenerated KC network)The 11 new failures are LRPs 20–31 m off a
serviceroad that bends ~90° within 20 m of the projection; the old single-segment heading matched by coincidence (Δ6°), the chord reads the bend (Δ96°). Spec-faithful, but a decode-count regression — if we'd rather keep them, the hard filter could acceptmin(Δchord, Δsegment)while scoring on the chord. Open to either.Results:
qa/results/477c043.parquet,qa/results/beardist.parquet(local, untracked).Test plan
cargo test— 71 pass🤖 Generated with Claude Code
https://claude.ai/code/session_01QStLGJkKBjXLPD8KkpsSjY