Cleanup raptor on trip access#7485
Conversation
# Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/McStopArrivals.java
…d test. # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/api/view/ArrivalView.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/c1/AccessStopArrival.java # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/api/view/ArrivalView.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/c1/AccessStopArrival.java # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/api/view/ArrivalView.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/c1/AccessStopArrival.java
…n DefaultRangeRaptorWorker
…ansit pass Remove the separate `routeTransitUsingOnBoardTripAccess()` method and fold on-board trip access handling into `boardAndAlightPattern`. On-board arrivals are now indexed by route index and stop position (via `OnBoardTripAccessPathsForRoute`), so they can be consumed inline at the correct stop during the regular route traversal instead of in a dedicated second pass. # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/McRangeRaptorWorkerState.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/McStopArrivals.java # Conflicts: # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/McRangeRaptorWorkerState.java # raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/McStopArrivals.java
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #7485 +/- ##
=============================================
+ Coverage 71.00% 71.04% +0.03%
- Complexity 20977 21023 +46
=============================================
Files 2345 2354 +9
Lines 87222 87299 +77
Branches 8637 8633 -4
=============================================
+ Hits 61931 62019 +88
+ Misses 22302 22295 -7
+ Partials 2989 2985 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note! I missed a few places when changing from |
bed846d to
bd1a2f0
Compare
jessicaKoehnke
left a comment
There was a problem hiding this comment.
I have found a few small things, otherwise very nice simplification!
| } | ||
|
|
||
| @Override | ||
| public RaptorTripScheduleReference tripScheduleReference(TripSchedule trip) { |
There was a problem hiding this comment.
should this be here when it only depends on the trip?
There was a problem hiding this comment.
Not sure I understand, can you explain?
There was a problem hiding this comment.
nothing from the RaptorRoutingRequestTransitData class or instance is actually needed to do the work. Only the trip itself, that's why I question the location of the method. For example, might it make more sense to put it in the TripSchedule itself?
There was a problem hiding this comment.
I will be on vacation next week, so I approve this for now so that you are not blocked. But I would like you to think of a good justification for where this method should be and maybe change it's location in one of your next PRs. (If there is a good reason for it being here, that might be fine, I would just like that choice to have been made consciously)
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorStartOnBoardAccess.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorStartOnBoardAccess.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripScheduleStopPosition.java
Outdated
Show resolved
Hide resolved
.../java/org/opentripplanner/raptor/rangeraptor/internalapi/OnBoardTripAccessPathsForRoute.java
Show resolved
Hide resolved
sigtot
left a comment
There was a problem hiding this comment.
Small docs suggestions otherwise looks good to me
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripScheduleStopPosition.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripScheduleStopPosition.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/api/model/RaptorTripScheduleStopPosition.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/internalapi/RoutingStrategy.java
Outdated
Show resolved
Hide resolved
.../main/java/org/opentripplanner/raptor/rangeraptor/multicriteria/arrivals/McStopArrivals.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/rangeraptor/DefaultRangeRaptorWorker.java
Show resolved
Hide resolved
Co-authored-by: Sigurd Totland <sigurdtotland@gmail.com>
Summary
This PR continues the Raptor API/SPI cleanup series (follows #7460). It refactors on-board trip access boarding to eliminate a dedicated second-pass through routes and improve encapsulation of trip-schedule lookup.
On-board trip access integration
routeTransitUsingOnBoardTripAccess()method fromDefaultRangeRaptorWorkerand RangeRaptorWorker interfaceOnBoardTripAccessPathsForRoutehelper, and consumed inline during the regularboardAndAlightPatterntraversalMcStopArrivals, keeping MC-specific logic containedRaptorTripScheduleStopPosition DTO
RaptorTripScheduleReference SPI
RaptorTripScheduleReferencein the SPI, backed byRaptorTransitDataProvider, to look up a trip schedule by its index (THIS IS CURRENTLY UNUSED, BUT WILL BE USED IN A FOLLOW UP PR)RaptorOnBoardAccessdirectlyNaming cleanup
tripIndex()→tripScheduleIndex()throughout (interface + all implementations)ArrivalsEventListenerMapper→McArrivalsEventListenerFactoryModuleTestDebugLogginghelper in favor of enabling debug logging uniformly across all module testsIssue
🟥 No issue for this. This is part of migrating vi pass-through in Raptor to new "chained" routers. This will enable pass-through to be used with Transit Group Priority and later also enable heuristics for via.
Unit tests
✅ Unit tests updated/added
Documentation
✅ JavaDoc updated
Changelog
🟥 No functionality changed
Bumping the serialization version id
🟥 No serialized model classes changed