Skip to content

Size the highway tile buffer in longitude degrees - #7

Open
jaideeppyne wants to merge 2 commits into
coding-parrot:mainfrom
jaideeppyne:fix/highway-tile-longitude-buffer
Open

Size the highway tile buffer in longitude degrees#7
jaideeppyne wants to merge 2 commits into
coding-parrot:mainfrom
jaideeppyne:fix/highway-tile-longitude-buffer

Conversation

@jaideeppyne

Copy link
Copy Markdown

tiles_for converts the 60 m tile buffer once, with the metres-per-degree of latitude, and applies that same degree value to longitude:

buffer_deg = TILE_BUFFER_METERS / 110_540
min_lng = min(point[0] for point in points) - buffer_deg

A degree of longitude spans 111320 * cos(latitude) metres, so the east-west buffer shrinks as you go north while the north-south one stays right:

latitude 8   ->  59.8 m
latitude 20  ->  56.8 m
latitude 30  ->  52.3 m
latitude 36  ->  48.9 m

max_match_distance_m is 45, so the shipped tiles are still correct today. I checked rather than assumed: across all 101 tiles and every shared edge, there is no segment within 45 m of an edge that is missing from the neighbouring tile. The margin at the top of the country is 3.9 m though, and at COORDINATE_SCALE 100000 a coordinate rounds to about 1.1 m, so it is thinner than the 15 m the manifest implies. Raising max_match_distance_m, or extending coverage further north, would start dropping matches with nothing to catch it.

A missed match here is silent. nationalHighwayRoute returns null and the report falls through to the city or state handoff, so a pothole on a national highway goes to a municipal authority that does not maintain it.

The fix sizes the two buffers separately and uses the highest latitude the way reaches, so it holds along the whole way.

I also added tests/national_highway_tile_buffer_test.py, since tile_buffer_m currently only appears in the builder and nothing checks the shipped packs against it. It asserts that every segment within max_match_distance_m of a shared edge is present on both sides, and that the east-west buffer still covers the match distance at the northernmost tile. It prints the real number, currently 48.9 m at latitude 36. I confirmed it fails when a tile loses its cross-edge geometry and when tile_buffer_m drops below the match distance, so it is not a test that passes by construction.

tools/build-national-highways.py --check still passes on the committed packs, 101 tiles and 680 refs. The change only affects the next rebuild, so no pack data moves here. national_highway_pack_test passes. nh_test and highway_contract_matching_test fail the same way before and after on my machine, missing dotenv and playwright.

Registered the test in tests/run-all.sh next to national_highway_pack_test.

I used Claude to write the sweep over the tiles and to draft this. I ran everything above myself.

tiles_for converted the 60 m tile buffer with the metres-per-degree of
latitude and applied it to longitude as well. A degree of longitude is
111320*cos(lat) metres, so the east-west buffer narrows towards the north
and reaches 48.9 m at latitude 36 against a declared 60 m.

Adds a test that the shipped tiles carry every neighbouring segment a
phone inside the tile could still match.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c319adb869

ℹ️ 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".

Comment thread tests/national_highway_tile_buffer_test.py Outdated
A segment that straddles a shared tile edge lies on it whatever the
distance from its endpoints, so the endpoint minimum let a boundary
crossing highway skip the check.
@jaideeppyne

Copy link
Copy Markdown
Author

Good catch from the Codex review, that was a real hole in the test and I have pushed a fix.

The check measured the distance from a segment's endpoints to the shared edge. A segment that crosses the edge lies on it no matter how far apart its ends are, so those were scored as far away and skipped, which is the opposite of what the test is for. Counting on the committed packs, 2216 segments cross a shared edge and 1120 of them had both endpoints more than 45 m away, so slightly over half of the boundary crossing highways were being ignored.

It now treats a straddling segment as distance 0. I checked the two versions against the same simulated regression, dropping exactly those crossing segments from a tile: the new one fails with e068n20 is missing a e070n20 segment 0.0 m past its east edge, the endpoint version passes. Shipped packs still pass, so the data really does carry those segments on both sides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant