Skip to content

Snap transit stops per access/egress mode (DMP-16462) - #3

Draft
mark-idleman wants to merge 1 commit into
patternsfrom
dmp-16462-per-mode-stop-snapping
Draft

Snap transit stops per access/egress mode (DMP-16462)#3
mark-idleman wants to merge 1 commit into
patternsfrom
dmp-16462-per-mode-stop-snapping

Conversation

@mark-idleman

Copy link
Copy Markdown
Collaborator

Background

DMP-16462. Stops were attached to the street network at a single point, found with a filter that every configured routing profile had to accept at once:

// Stops must be connected to the networks of all the modes
List<DefaultSnapFilter> snapFilters = getProfiles().stream().map(...)
gtfsReader.connectStopsToStreetNetwork(e -> {
    for (DefaultSnapFilter snapFilter : snapFilters) {
        if (!snapFilter.accept(e)) return false;
    }
    return true;
});

Nothing near an airport platform satisfies that — footways ban cars, terminal roadways ban pedestrians — so the attachment landed on whatever distant service road allowed everything, or was dropped entirely with no error. At SLC that made the TRAX platform's access walk 2,547 m instead of 8 m, past our 24-minute cap, so the airport recorded zero boardings for all of 2025_Q4.

This is upstream code (547215973, Michael Zilske, 2023), unchanged in upstream 8.0 and 9.0 — upgrading does not fix it. It's near-free upstream, whose test config has 2 profiles; our config has 10, including truck, small_truck and car_freeway, which no transit query would ever request as an access mode.

The intersection existed for a reason: there is exactly one ptToStreet entry per stop, one shared attachment for every access/egress mode. Given that, the filter genuinely did have to satisfy every profile. This PR removes the constraint rather than narrowing it.

Description

One attachment per mode instead of one attachment for all modes. A query looks up the attachment belonging to its own access/egress profile, so a walking leg uses the platform footway while a car leg can use the kerb — same stop node, different doors into it.

  • gtfs.stop_snap_profiles (comma-separated, first entry primary) selects which profiles get snapped. Defaults to foot alone.
  • The primary profile alone decides stop node identity, so the transit graph doesn't depend on which modes happen to be configured. This preserves today's merging of co-located stops (including across feeds) onto a shared stop node — now keyed on walking geometry.
  • A request naming an unsnapped profile falls back to the primary attachments with a one-time warning, rather than failing. Non-walk modes still work wherever the attachment node is shared with a road.
  • An unknown profile name is rejected before the store is created. Previously getProfile() returned null and NPE'd inside createWeighting, surfacing as "Is your GTFS file valid?".
  • Unattached stops and street-node collisions are now logged with counts. The silent orphan path is what let this run a full season undetected.

GraphExplorer resolves its two maps once in the constructor instead of reaching through gtfsStorage per edge.

Reviewer notes

Two things worth a close look:

  1. Stop-node merging changes. Identity moves from the all-modes snap to the foot snap, so which stops merge changes in every region, not just at airports. At SLC the platform and bus bays are probably collapsed onto that one service road today and will separate. Expect transfer counts to move; that's the number to watch, more than mode share.
  2. streetToPt is one-to-one. If two stops share their nearest street node for a mode, only the first is discoverable from the street side for that mode (counted in the log line). Today's merging hides this. Fixing it properly means an IntObjectHashMap<int[]> and a multi-successor getAdjNode(), which touches search semantics — deliberately left out of scope. Nothing requests car access today.

Store format

Breaking: per-profile pt_to_street_<profile> / street_to_pt_<profile> plus a stop_snap_profiles manifest. Loading an older store fails with an explicit message instead of FileNotFoundException. A router image must not be rolled ahead of a graph rebuild — relevant because graphhopper_runner_image_tag and graphhopper_builder_image_tag are bumped independently in the model repo.

Testing

PerModeStopSnappingIT (5 tests, all passing) covers: the foot default, independent per-profile attachments, round-trip through flush/reload, fallback for an unsnapped profile, and rejection of an unknown profile name.

Regression-checked against a clean patterns worktree over GraphHopperGtfsIT, GraphHopperMultimodalIT, AnotherAgencyIT, RealtimeIT, FreeWalkIT, ExtendedRouteTypeIT:

failures
patterns baseline 21
this branch 21 — identical set

Zero new failures, zero fixed. The 21 are pre-existing on patterns and unrelated. One regression was found and fixed during this work: testCustomProfileAccess requests car_custom as its access profile and initially hard-failed, which is what motivated the fallback.

Full mvn test-compile passes across all modules.

Not yet done

  • No real-world validation. The pass condition in the ticket is the access walk at 40.78427, -111.98336 dropping from 2,547 m to ~8 m (w1141358400) — needs a southwest/2025_Q4 pilot build and a live router.
  • Nationwide blast radius on stop merging unmeasured.
  • Model-repo side is not part of this PR: gtfs.stop_snap_profiles: foot in default_gh_config.yaml, a v102 base image, and image tag bumps in both usa_base.yaml.tmpl:111-112 and regional_base.yaml.tmpl:80-81 (the latter is what southwest/2025_Q4 actually inherits).

Stops were attached to the street network at a single point found with a filter that
every configured routing profile had to accept at once. Nothing near an airport
platform satisfies that -- footways ban cars, terminal roadways ban pedestrians -- so
the attachment landed on whatever distant service road allowed everything, or was
dropped entirely with no error. At SLC that put the TRAX platform's walk access at
2,547 m instead of 8 m, past the 24-minute access cap, so the airport recorded zero
boardings for 2025_Q4.

Each stop now gets one attachment per mode instead of one attachment for all modes.
A query looks up the attachment belonging to its own access/egress profile, so a
walking leg uses the platform footway while a car leg can use the kerb.

- gtfs.stop_snap_profiles (comma-separated, first entry primary) selects which
  profiles are snapped. Defaults to "foot" alone.
- The primary profile alone decides stop node identity, so the transit graph does not
  depend on which modes happen to be configured. This preserves the existing merge of
  co-located stops onto a shared stop node, now keyed on walking geometry.
- A request naming an unsnapped profile falls back to the primary attachments with a
  warning rather than failing.
- An unknown profile name is rejected before the store is created; previously it would
  NPE inside createWeighting and be reported as an invalid GTFS feed.

Changes the graph store format: per-profile pt_to_street_<profile> /
street_to_pt_<profile> plus a stop_snap_profiles manifest. Loading a store written
before this change fails with an explicit message, so a router image must not be
rolled ahead of a graph rebuild.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LOGGER.warn("Feed {}: {} stops could not be attached to the street network for any of the profiles"
+ " {}, so they are reachable only by stop id.", id, unattachedStops, snapFiltersByProfile.keySet());
}
if (sharedAttachments > 0) {

@BNewborn BNewborn Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this true? Does a stop/profile sharing a node mean one of the stop/profile points becomes unroutable? Claude suggests this is only for car / access, but we dont use that ATM. Maybe its worth just removing this warning if I'm understanding this all correctly?

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