Upstream merge 2026-05-07#301
Open
leonardehrenfried wants to merge 40 commits intoibi-dev-2.xfrom
Open
Conversation
Introduce TripPatternGeometry, a package-private Serializable helper that encapsulates the compressed per-hop geometries previously held directly on TripPattern together with a precomputed int[] cumulative distance table (meters) built once at graph build. It holds its own StopPattern reference so that accessors take only stop indices, and exposes distanceBetween(board, alight), geometryBetween(board, alight), hopGeometry(int) and concatenatedGeometry() as O(1) / stateless lookups. ScheduledTransitLeg now obtains both distanceMeters and legGeometry in O(1) via tripPattern.distanceBetween(board, alight) and tripPattern.geometryBetween(board, alight), replacing the per-leg haversine sum that was recomputed on every construction path (initial mapping, alternative-legs GraphQL fetcher, alert/emission/fare decorators, leg-reference resolution, real-time rebuilds). The single-method utility LegConstructionSupport.extractSubGeometry is inlined into TripPatternGeometry.geometryBetween and the class is removed. Cumulative distances are summed in double and rounded to the nearest meter only when writing each entry of the int[] table, bounding the rounding error of any distanceBetween query to at most 1 m regardless of leg length. Tests and snapshots are updated for the new meter-precision output, and a TripPatternGeometryTest covers the two factory paths (with and without hop geometries), the distanceBetween contract (within 1 m of exact haversine, additive across cumulative entries, zero rounding drift over a 20-stop pattern), the geometry accessors (compressed round-trip, straight-line synthesis, concatenated pattern geometry), and degenerate single-stop / empty-hop / board==alight edge cases.
Replace the StopPattern field on TripPatternGeometry with a boolean fromShape flag. The factory now materializes the shapeless fallback at construction: when no shape data is provided, it builds a 2-point straight-line LineString between each pair of consecutive stops and compresses it through CompactLineStringUtils, the same path used for real shape data. The stopPattern argument is consumed by the factory only and not retained on the instance. hopGeometry(int) simplifies to a single uncompactLineString call — no more null-branching to read stop coordinates on the fly. The null return of concatenatedGeometry() is preserved via !fromShape, keeping the GraphQL null-sentinel for shapeless patterns. Memory impact: shapeless patterns pay ~12–16 bytes per hop for the compressed synthesized hop (plus a byte[] header). Real-shape patterns are unchanged.
Coarsen Units.speed (step 0.05 below 2 m/s) and Units.normalizedFactor (step 0.1 below 3.0, 0.5 in [3, 10), 1.0 above) so close-but-distinct client-supplied values share a single canonical representation. This reduces the number of distinct StreetRelevantOptions tuples that trigger a RaptorTransferIndex.createRequestScope rebuild on first touch — the root cause of the rollout-time P95 spike observed when many clients vary walk speeds within a 0.01 m/s window. Apply the bucketing centrally in Units rather than only in the transfer cache key, so transit pre-computation, access, egress, direct paths and boarding/alighting durations are internally consistent for every request. Realign the WalkPreferences and VehicleWalkingPreferences raw default walk speed from 1.33 to 1.35 (the bucketed value) so DEFAULT.x equals of().build().x, and update the corresponding JavaDoc, GraphQL schema descriptions and generated user docs. Switch ItineraryFilterPreferences.groupSimilarityKeepOne and groupSimilarityKeepThree from Units.reluctance to Units.ratio. These fields are similarity ratios in [0, 1], not unbounded reluctances; the ratio normalisation preserves their original 3-decimal precision (so the documented 0.85 / 0.68 defaults stay put rather than silently shifting to 0.9 / 0.7 under the new reluctance grid) and rejects out-of-range values with IllegalArgumentException instead of silently accepting nonsensical inputs like 2.0.
Drop the 2-decimal tier (was used for absolute values < 2.0); the new bucket grid carries only 1 decimal of precision below 10.0, so format to match. Add negative-value tests for Units.normalizedFactor and regenerate RouteRequest.md and the transmodel schema.
ratio is a scalar, not meters per second squared (which was copied from acceleration).
Align the GTFS graph builder with the NeTEx ServiceLinkMapper: when a trip has no shape_id, fall back to straight lines between consecutive stops instead of returning null. The other paths in createHopGeometries already used this fallback for invalid or unmatchable shape data; this extends it to the "no shape data at all" case so the method's return type is always non-null.
…eometries Always generate hop geometries in the GTFS graph builder
…/jaxb-ri-monorepo Update dependency org.glassfish.jaxb:jaxb-runtime to v4.0.8
Use banker's rounding for the bucket-grid rounding so the utility does not introduce systematic bias for callers that may sum or average rounded values. BigDecimal pinning is still required to detect ties reliably regardless of IEEE-754 representation.
…eting Coarsen speed and reluctance values at the request level
PR opentripplanner#7571 makes the GTFS graph builder always return a non-null hop geometry list (straight lines between consecutive stops when shape data is missing), matching what the NeTEx ServiceLinkMapper already did. With both importers feeding non-null geometries into TripPatternGeometry, the fromShape boolean and the conditional null-return from concatenatedGeometry() exist only to model the "no shape" sentinel for real-time-added patterns. Drop them: getGeometry() now returns null only for degenerate patterns with no hops, and shapeless or real-time-added patterns expose their straight-line concatenation through GraphQL. Also drop the matching null-tracking ternary in TripPatternBuilder's copy constructor, and rename the test that pinned the null-return.
…ometry # Conflicts: # application/src/test/java/org/opentripplanner/routing/algorithm/mapping/__snapshots__/ElevationSnapshotTest.snap
The merge of dev-2.x brought in PR opentripplanner#7569 (units coarser bucketing), which shifted timings and costs across all itinerary snapshots and overlapped with the whole-meter distance rounding from this PR. Both the literal merge conflict and the auto-merged adjacent regions are resolved by regenerating the snapshot, so that it carries both the bucketed timing/cost values from dev-2.x and the whole-meter transit leg distances from this PR.
- Add an IllegalArgumentException precondition in TripPatternGeometry.of if the supplied hopGeometries list does not match the number of hops implied by the stop pattern. Catches caller bugs eagerly instead of silently producing an inconsistent cumulative-distance table. - concatenatedGeometry() no longer returns null. For degenerate patterns with no hops (one stop or fewer) it returns an empty but non-null LineString — symmetric with geometryBetween(i, i). TripPattern's getGeometry() drops its @nullable annotation accordingly. The TripPatternTest.sameAs check that did .copy().withStopPattern(differentSize) now also clears hopGeometries to avoid the new precondition; this was a latent inconsistency in the test fixture rather than in production code (no production caller mutates a copied TripPatternBuilder's stop pattern without also resetting hop geometries — verified across all five withStopPattern call sites and the single TripPattern.copy() caller).
…dule Fix RAPTOR path building for flex patterns with duplicated stops
…-geometry Precompute cumulative pattern distance for O(1) leg distance lookup
…-map Reduce nearby area stop search allocation pressure
miles-grant-ibigroup
requested changes
May 7, 2026
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
I'm not sure I'm comfortable merging this! I'm seeing the following issue with timeframes:
16:35:10.304 INFO [main] (GtfsModule.java:328) Reading entity: org.onebusaway.gtfs.model.Timeframe
16:35:20.008 WARN [main] (TransitDataImportBuilder.java:290) Limiting transit service days to time period: [2025-05-07, 2029-05-07]
16:35:20.107 INFO [main] (TransitDataImportBuilder.java:311) Limiting transit service days to time period complete.
16:35:20.170 ERROR [main] (OTPMain.java:60) An uncaught error occurred inside OTP: The value is not in range[0.05, 1.7976931348623157E308]: 0.0
java.lang.IllegalArgumentException: The value is not in range[0.05, 1.7976931348623157E308]: 0.0
at org.opentripplanner.utils.lang.DoubleUtils.requireInRange(DoubleUtils.java:112)
at org.opentripplanner.utils.lang.DoubleUtils.requireInRange(DoubleUtils.java:106)
at org.opentripplanner.ext.flex.trip.UnscheduledTrip.<init>(UnscheduledTrip.java:74)
at org.opentripplanner.ext.flex.trip.UnscheduledTripBuilder.buildFromValues(UnscheduledTripBuilder.java:50)
at org.opentripplanner.ext.flex.trip.UnscheduledTripBuilder.buildFromValues(UnscheduledTripBuilder.java:8)
at org.opentripplanner.transit.model.framework.AbstractBuilder.build(AbstractBuilder.java:33)
at org.opentripplanner.ext.flex.FlexTripsMapper.createFlexTrips(FlexTripsMapper.java:42)
at org.opentripplanner.gtfs.graphbuilder.GtfsModule.buildGraph(GtfsModule.java:165)
at org.opentripplanner.graph_builder.GraphBuilder.run(GraphBuilder.java:222)
at org.opentripplanner.standalone.OTPMain.startOTPServer(OTPMain.java:142)
at org.opentripplanner.standalone.OTPMain.main(OTPMain.java:55)
16:35:20.182 INFO [server-shutdown-info] (OtpStartupInfo.java:47) OTP SHUTTING DOWN - Build Street & Transit Graph - Version: 2.10.0-SNAPSHOT, ser.ver.id: 253, commit: d2d3cc9dcd343a31eb026c0cf027900fc0963ebd, branch: upstream-merge-2026-05-07
The issue is not appearing with the previous build
Author
|
From the stack trace it looks unrelated to time frames. It seems to happen because they put 0 as the duration factor which means that the flex leg takes 0 seconds. |
|
It probably is invalid data, but the previous build supports it? Or is it wrong to support this |
Author
|
Can you please double check if it's a new problem? |
Author
|
Or send the feed so I can check myself. |
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
3 participants
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.
This contains the fix for the Sandy Flex exception.