Skip generated cells with no assigned ways - #112
Closed
FrogAi wants to merge 1 commit into
Closed
Conversation
This was referenced Aug 9, 2026
Contributor
Author
|
Closing this because omitting empty tiles changes runtime semantics from a loaded, authoritative empty tile to a missing/unloaded tile. On current runtime paths, that changes retry/state behavior and can leave prior road-derived state in use. Empty-tile generation should not change until the runtime contract for intentionally empty coverage and stale-state clearing is defined. |
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
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.
Summary
GenerateEmptyFiles=falsebefore allocating or serializing an empty tile.GenerateEmptyFiles=true.Motivation
The current empty-cell check does not inspect ways. It first requires the requested bounds to contain a cell, then asks whether those same bounds overlap the cell. The overlap is guaranteed by the preceding containment check, so
GenerateEmptyFiles=falsecannot suppress any selected cell.The generator only knows whether a cell is empty after applying its existing way-assignment rule. This change moves that unchanged assignment loop before Cap'n Proto allocation and skips the cell when the resulting way list is empty and empty-file generation is disabled.
Behavior
A focused generation fixture selected nine quarter-degree cells containing three populated cells and six cells with no assigned ways:
68813e05c57618eGenerateEmptyFiles=false: total cells writtenGenerateEmptyFiles=false: zero-way cells writtenGenerateEmptyFiles=true: total cells writtenAll six retained empty files in the
truecase decoded as valid zero-way tiles. The three populated files kept the same paths, expected way counts, and byte-identical packed contents across flag modes."Empty" remains defined by the existing inclusive way-box overlap check, including the configured overlap margin. This does not change boundary assignment or replace the current AABB model with exact road geometry.
Validation
The focused oracle used a real Delaware PBF normalized through the repository's
osmium add-locations-to-wayspreprocessing step:go test ./...,go vet ./..., andgo build ./...with Go 1.25.1 on Linux/amd64.make buildfor exact commitc57618ebde71678997eb0fa8933d98a401e2c7cd.Compatibility
GenerateEmptyFiles=trueretains the current selected-cell output.GenerateEmptyFiles=falseintentionally stops writing newly generated cells with no assigned ways.mapd generateis no longer idempotent for a reused output directory. Skipping the write means any previously generated tile for a now-empty cell survives, so the output becomes the union of every PBF ever generated into that directory — including stale populated tiles, not just the stale empty ones. Reproduced twice independently: regenerating with a source that no longer has ways for a cell leaves the old tile in place, and the daemon then serves deleted roads as authoritative. Base overwrote it with a zero-way tile. Reachable by default, since generate bounds cover the whole world.main.gocontinues whenFindWaysAroundPositionerrors, skipping the block that is the only placeCurrentWay,NextWays,Curvatures, andTargetVelocitiesare assigned. Base loaded the empty tile,GetCurrentWayreturnedfail, and the speed limit cleared to 0; head leaves the previous road'swayNameand speed limit republishing at 20 Hz for as long as you are in that cell, withspeedLimitSuggestedSpeedstill clamping cruise — effectively an unconditional hold-last-seen. It also flipstileLoadedfrom true to false for genuinely road-free cells and logs a warning per GPS fix. Reachability is bounded byscripts/filter_planet.sh, which strips the planet to 12 highway classes before generation, so an "empty" cell has no motorway through residential road; in a Delaware run, zero cells were drivable-yet-empty. Sparse regions are unproven either way. Fixing this means touchingmain.go(drop thecontinue, or only continue on a real I/O fault rather than ENOENT), which is out of scope here and interacts with #114.scripts/upload_offline.shusesrclone copyrather thansync, and client extraction never deletes, so the size win is not delivered to downloaders until the bucket is purged. Once it is, the download progress counter stalls below 100% on road-free groups, becausecountFilesForBoundscounts group rectangles arithmetically while 404s skip the downloaded-file increment.Merge order with #113 — read before resolving
Merging #113 with this PR conflicts, and the obvious resolution silently corrupts every generated tile. This PR relocates the way-assignment loop; #113 replaces that loop with a bucketed version. Git reports a conflict at the bucketed block, but this PR's relocated loop auto-merges about 20 lines above the conflict markers, where it is invisible in the conflict view. Taking the incoming block therefore appends to
area.Waystwice. Measured on the naive resolution: 33 tiles, 151,994 ways, 75,997 duplicate way IDs, 2x tile size — withgo build,go vet, andgo testall green, so CI will not catch it.Correct resolution, built and verified byte-identical to this PR alone: delete this PR's relocated loop and its gate, keep #113's bucketed block after the
Containscheck, then placeif len(area.Ways) == 0 && !s.GenerateEmptyFiles { continue }after it and before the capnp arena allocation. #97 merges cleanly with this PR (output byte-identical); stacking all three adds only a self-diagnosingstdmath "math"import-alias conflict.