Skip to content

Decode transit (level 3) graph tiles - #16

Closed
Archdoog wants to merge 11 commits into
stadiamaps:mainfrom
Rallista:feat/transit-tile-decode
Closed

Archdoog wants to merge 11 commits into
stadiamaps:mainfrom
Rallista:feat/transit-tile-decode

Conversation

@Archdoog

@Archdoog Archdoog commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

GraphTileView::try_from rejected all tiles above level 2.
Transit tiles have the same on-disk sections in the same order as the other levels,
and the decoder already reads all five transit sections.
This PR removes the guard for level 3 and tests the decoder against transit tiles
that Valhalla 3.8.2 built.

Changes

  1. Fixture. valhalla-graphtile/fixtures/andorra-transit-tiles has one transit tile
    and the local tile under it. Each tile is less than 4 KB.
    Valhalla 3.8.2 built the tiles from synthetic input.
    The input was a 3 by 4 street grid in Andorra la Vella.
    A GTFS feed added 4 stops, 1 bus route, and 3 trips.
    One trip has a frequencies.txt entry.
    The PR does not include the build script or the inputs.
    The tests hard-code the input values that they check.
  2. Level lookup. tile_hierarchy::tile_level returns the TileLevel for a level number.
    It looks in STANDARD_LEVELS and TRANSIT_LEVEL, and it returns None for other levels.
    GraphId::file_path uses it. For levels 4 to 7, file_path returns
    InvalidGraphIdError::Level and does not panic on an index out of bounds.
  3. Transit records. TransitDeparture, TransitStop, TransitRoute, TransitSchedule,
    and TransitTransfer get bit fields and accessors.
    The layout follows the
    transit*.h headers
    of Valhalla.
    The record sizes do not change.
    end_time and frequency return None for a fixed departure,
    because a fixed departure keeps its elapsed time in the end time bits.
  4. Decoder. try_from rejects only the levels that tile_level does not know.
    Thus bin_index_xy does not panic on a transit tile.
    The TODO notes about one stop IDs are gone.
    Valhalla builds those lookup maps in memory after it loads a tile (AssociateOneStopIds).
    They are not in the tile bytes.

Tests

  • Each transit tile in the fixture decodes.
    The header counts (departures, stops, routes, schedules, and transfers)
    match the slice lengths.
  • The stop names, Onestop IDs, and platform coordinates match stops.txt.
  • The route short name, long name, description, operator, type, and colors
    match routes.txt and agency.txt.
  • All 9 departures match stop_times.txt, trips.txt, and frequencies.txt.
    This includes the headsigns, the accessibility flags, and the frequency fields.
  • The schedule has the Monday to Friday mask from calendar.txt,
    and its day bits repeat each week.
  • The local tile from the same build decodes.
    Its 8 transit connection edges end at the 4 egress nodes.
  • DirectoryGraphTileProvider follows the opposing edges between the transit tile
    and the local tile.
  • bin_index_xy and tile_level work for a transit tile.
  • Unit tests check the record sizes and the fixed and frequency departure bits.
  • Insta snapshots hold the stops, the route, the departures, and the schedule.

CI

ubuntu-latest is now Ubuntu 26.04, which has no qemu-user-static package,
so the s390x test job fails on main.
#18 fixes the job. This PR merges the branch of #18, so the s390x job runs here.
If #18 merges first, the .cargo/config.toml and rust.yml changes leave this diff.

Manual testing

Local results:

  • cargo fmt --check, cargo nextest run (113 tests), and cargo test --doc pass.
  • cargo clippy --all-targets shows no new warnings when compared with main.

I did not run Miri locally.

Known gap: bounding circles

Valhalla 3.8.0 and later write bounding circles into level 2 tiles by default.
See valhalla/valhalla#5103.
valhalla-graphtile does not read this section yet,
so it fails to decode these tiles with LeftoverBytesAfterReading.
The fixture build sets mjolnir.data_processing.build_bounding_circles to false.
This PR does not fix the gap.

🤖 Generated with Claude Code

Archdoog and others added 5 commits October 28, 2026 15:31
Add one transit (level 3) tile and the local (level 2) tile under it.
Valhalla 3.8.2 built the tiles from synthetic input.
The input was a 3 by 4 street grid in Andorra la Vella.
A GTFS feed added 4 stops, 1 bus route, and 3 trips.
One trip has a frequency entry.
The build turned off bounding circles,
because valhalla-graphtile cannot decode them yet.
Each tile is less than 4 KB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tile_hierarchy::tile_level returns the definition of a standard level
or the transit level, and None for all other levels.

GraphId::file_path uses it.
For levels 4 to 7, file_path returns InvalidGraphIdError::Level
and does not panic on an index out of bounds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Give TransitDeparture, TransitStop, TransitRoute, TransitSchedule,
and TransitTransfer bit fields and accessors.
The layout follows the transit*.h headers of Valhalla.

A fixed departure keeps its elapsed time in the bits
that a frequency departure uses for the end time.
The end_time and frequency accessors return None for a fixed departure.
The Debug output of TransitDeparture shows only the fields
of its departure type, because the other bits can hold stale data.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GraphTileView::try_from rejected all tiles above level 2.
Transit tiles have the same sections in the same order
as the other levels, and the decoder reads all five transit sections.
The decoder now rejects only levels that tile_level does not know.

bin_index_xy uses tile_level, so it does not panic on a transit tile.
Remove the TODO notes about one stop IDs.
Valhalla builds those lookup maps in memory after it loads a tile.
They are not in the tile bytes.

The tests use the Andorra transit fixture. They check that:

- Each transit tile decodes, and the header counts match the slice lengths.
- The stops, the route, the departures, and the schedules
  match the GTFS input.
- The local tile from the same build decodes,
  and its transit connection edges end at egress nodes.
- A directory tile provider follows the opposing edges
  between the transit tile and the local tile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ubuntu-latest runner image is now Ubuntu 26.04.
In this release, qemu-user-static is a virtual package with no install candidate,
so the s390x job fails before it runs the tests.

The qemu-user package provides /usr/bin/qemu-s390x on Ubuntu 24.04 and 26.04.
Install qemu-user and use qemu-s390x as the cargo runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread valhalla-graphtile/src/graph_tile.rs Resolved
Comment thread .cargo/config.toml Resolved
Archdoog and others added 3 commits October 28, 2026 15:53
Address the review comments on the level lookup and the bin_index_xy docs.

ValhallaLevel has one variant for each level that Valinor knows
(Highway, Arterial, Local, and Transit).
The discriminant is the level number.
TryFrom<u8> checks a level number without the LazyLock statics,
and definition() returns the TileLevel without an Option.

GraphTileView::try_from stores the checked level in the view.
Thus bin_index_xy does not need an expect.
GraphId::file_path uses the same check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ubuntu-latest runner image is now Ubuntu 26.04.
In this release, qemu-user-static is a virtual package with no install candidate,
so the s390x job fails before it runs the tests.

The qemu-user package provides /usr/bin/qemu-s390x on Ubuntu 24.04 and 26.04.
Install qemu-user and use qemu-s390x as the cargo runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This reverts commit bd4e617.
The change to qemu-user is in stadiamaps#18,
so this PR has only the transit changes.
The s390x test job fails on this PR until stadiamaps#18 merges.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread valhalla-graphtile/src/graph_tile.rs Resolved
Archdoog and others added 3 commits October 28, 2026 16:42
Merge the branch of stadiamaps#18, so the s390x job runs on this PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Apply the review suggestions for the two docstrings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The enum has a fixed set of levels.
The TileLevel statics let a user define a custom level.
Thus we keep the statics and the tile_level lookup.

This reverts 2f8d564 and the enum part of bc10cad.
The bin_index_xy panic doc stays short,
and it points to STANDARD_LEVELS and TRANSIT_LEVEL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@ianthetechie ianthetechie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Voted to defer this for now but leaving the comments in case we revisit later.

/// Use [`ValhallaLevel::try_from`] to check a level number from a graph ID or a tile header.
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[repr(u8)]
pub enum ValhallaLevel {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Discussed on the call; let's scrap this for now since there are actually 8 (0-7) levels possible and this could be changed / extended either by Valhalla or by a Valinor user in the future (I could totally see adding your own tile level with proprietary data for example). Using an enum encourages exhaustive matching (normally good!) but then it's imposing a bit too strict of an interpretation on downstream code for now.

Comment on lines -487 to -489
// TODO: Stop one stops(?)
// TODO: Route one stops(?)
// TODO: Operator one stops(?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These were real placeholders! See https://github.com/valhalla/valhalla/blob/983da51337573b43ac8b252fdaa3139922b8f3cd/valhalla/baldr/graphtile.h#L1056.

If this isn't necessary, the PR needs to explain it.

Comment on lines -878 to -882
// TODO: Transit info
// - Stop one stops(?)
// - Route one stops(?)
// - Operator one stops(?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Figure out where these are supposed to be ;)

//! # Transit data structures
//!
//! TODO: The authors don't actively use transit data structures, so these are just (correctly sized) stubs for now.
//! These records are in transit (level 3) tiles.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reword

Comment on lines +4 to +6
//! Text offsets point into the text list of the same tile.
//! Raw enum values (such as the route type) use the numbering in Valhalla's
//! [`graphconstants.h`](https://github.com/valhalla/valhalla/blob/b77d5987ed13562454681fc06a81b39d33d97abf/valhalla/baldr/graphconstants.h).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation details don't belong here; link main branch, not versioned blobs unless referencing some very specific behavior at a point in time.

into = bit_twiddling_helpers::conv_u64le::into_inner
)]
#[derive(FromBytes, IntoBytes, Immutable, Unaligned)]
struct DepartureFirstBitfield {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cross check this against baldr/transitdeparture.h

@Archdoog Archdoog closed this Oct 29, 2026
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.

2 participants