-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Direct transit search #7145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-2.x
Are you sure you want to change the base?
Direct transit search #7145
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #7145 +/- ##
=============================================
- Coverage 72.14% 71.97% -0.17%
- Complexity 21045 21112 +67
=============================================
Files 2292 2305 +13
Lines 85036 85569 +533
Branches 8473 8526 +53
=============================================
+ Hits 61349 61592 +243
- Misses 20713 20993 +280
- Partials 2974 2984 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
81feadd to
6081246
Compare
raptor/src/main/java/org/opentripplanner/raptor/direct/api/RaptorDirectTransitRequest.java
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/direct/api/RaptorDirectTransitRequest.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/direct/service/DirectTransitSearch.java
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/spi/RaptorSlackProvider.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java
Outdated
Show resolved
Hide resolved
...a/org/opentripplanner/routing/algorithm/raptoradapter/transit/AccessEgressWithExtraCost.java
Outdated
Show resolved
Hide resolved
...st/java/org/opentripplanner/routing/api/request/preference/DirectTransitPreferencesTest.java
Outdated
Show resolved
Hide resolved
...main/java/org/opentripplanner/standalone/config/routerequest/DirectTransitRequestConfig.java
Outdated
Show resolved
Hide resolved
raptor/src/main/java/org/opentripplanner/raptor/api/request/RelaxedLimitedTransferRequest.java
Outdated
Show resolved
Hide resolved
...st/java/org/opentripplanner/raptor/moduletests/M02_DirectTransitWithTripsInSearchWindow.java
Outdated
Show resolved
Hide resolved
| * <p> | ||
| * The direct transit search should include trips within the cost limit | ||
| */ | ||
| public class M04_RelaxedCostLimit implements RaptorTestConstants { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test description is incorrect. This test is testing departure time and arrival time for optimality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand the comments on each of the test methods - they seem to focus on arrive/departure times, while the class objective talk about cost. Are arrival & departure times considered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I understand what you mean and I tried to clarify the comments. The search will include results if they are optimal on arrival or departure or if they are within the "cost limit". These tests are set up in such a way that arrival and departure times should not influence whether the trip under test gets included or not.
Maye I should just remove the reference to arrival and departure times in the comments.
...st/java/org/opentripplanner/raptor/moduletests/M01_DirectTransitWithRoutesWithinRelaxC1.java
Outdated
Show resolved
Hide resolved
|
You need to resolve the conflicts. |
raptor/src/test/java/org/opentripplanner/raptor/moduletests/M04_RelaxedCostLimit.java
Outdated
Show resolved
Hide resolved
raptor/src/test/java/org/opentripplanner/raptor/RaptorArchitectureTest.java
Outdated
Show resolved
Hide resolved
raptor/src/test/java/org/opentripplanner/raptor/moduletests/package-info.md
Outdated
Show resolved
Hide resolved
...ntripplanner/routing/algorithm/raptoradapter/transit/mappers/DirectTransitRequestMapper.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/opentripplanner/routing/api/request/preference/DirectTransitPreferences.java
Show resolved
Hide resolved
…m/raptoradapter/transit/mappers/DirectTransitRequestMapper.java Co-authored-by: Thomas Gran <[email protected]>
| @Override | ||
| public String toString() { | ||
| return ToStringBuilder.of(DirectTransitPreferences.class) | ||
| .addBool("enabled", enabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we only log this when it doesn't match the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be helpful to log it even when it is default (false) since this will cause the direct transit search to be disabled.
| ) | ||
| .withCostRelaxFunction( | ||
| c | ||
| .of("costRelaxFunction") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What did we decide with this naming? I can't remember anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to leave it like this to keep it consistent with the transit group priority. If we decide to change this to "costLimitFunction" we will do that together with the transit group priority feature.
|
Formatting is broken. |
Summary
This PR adds support for a complimentary search that is run in addition to the regular raptor search. This new search extends the search result with "direct" transit paths, that is paths with a single transit leg. In contrast to raptor it returns paths even if they are not optimal according to any criteria. It can be configured to run with access/egress or only for stop to stop searches.
The main use case for this additional search is to show slower journeys such as regional trains, slower buses etc that are not included in the main raptor search. See the highlighted journey in the example below, using an ordinary raptor search it will never show up in a search results because the previous journey is better in all criteria.
Issue
Closes #6977. See issue for detailed use cases.
Implementation
The search is implemented in the raptor module and runs on the raptor transit data in order to take advantage of the cost calculation and the itinerary mapping.
Configuration
This feature is currently not exposed through any api and is configurable through
router-config.json. It takes three configuration parameters:costRelaxFunctionIn order to exclude really bad paths there is a cost function that is used to reject paths that are much more expensive than the cheapest path.
extraAccessEgressCostFactorWhen using this search in an area with lots of transit options such as a city center it can produce too many results where one walks for a while, takes a short transit and then walks to the destination. Having too many of these alternatives will not be useful to travelers. They can be limited using an extra cost on accesses and egresses that is specific for this search.
maxAccessEgressDurationThe maximum length of access/egress, can also be configured to not use access/egress at all. In that case it will only be used when searching to and from a stop or station.
Bumping the serialization version id
Yes, updating the route request.