Multiplicative stairs and turn reluctance #6783
Conversation
…e on top of regular reluctance
miklcct
force-pushed
the
multiplicative-stairs-and-turn-reluctance
branch
from
fb933c4 to
4c16517
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev-2.x #6783 +/- ##
==========================================
Coverage 71.99% 71.99%
- Complexity 19432 19437 +5
==========================================
Files 2100 2100
Lines 78748 78759 +11
Branches 7961 7963 +2
==========================================
+ Hits 56697 56705 +8
- Misses 19245 19247 +2
- Partials 2806 2807 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@miklcct I changed This fixes the #6662 as fare as I understand? If not, what is it that is not fixed @leonardehrenfried / @miklcct ? |
Thanks for your correction. To completely fix #6662 a new input on the GTFS GraphQL API may also be required as well. |
| @@ -117,7 +117,9 @@ private GraphQLObjectType createGraphQLType() { | |||
| .field( | |||
| GraphQLFieldDefinition.newFieldDefinition() | |||
| .name("stairsReluctance") | |||
| .description("Used instead of walkReluctance for stairs.") | |||
| .description( | |||
| "A multiplier to specify how bad walking on stairs is, compared to walking on flat ground." | |||
There was a problem hiding this comment.
I think we should clarify that this is used in addition to the walk reluctance.
There was a problem hiding this comment.
I think this is still unchanged.
.../main/java/org/opentripplanner/routing/api/request/preference/VehicleWalkingPreferences.java
Outdated
Show resolved
Hide resolved
...cation/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java
Outdated
Show resolved
Hide resolved
...ion/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/org/opentripplanner/standalone/config/routerequest/VehicleWalkingConfig.java
Outdated
Show resolved
Hide resolved
...tion/src/main/java/org/opentripplanner/street/model/edge/StreetEdgeReluctanceCalculator.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java
Show resolved
Hide resolved
# Conflicts: # application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java # application/src/test/java/org/opentripplanner/routing/algorithm/mapping/__snapshots__/ElevationSnapshotTest.snap # application/src/test/java/org/opentripplanner/routing/algorithm/mapping/__snapshots__/TransitSnapshotTest.snap # application/src/test/resources/speedtest/travelSearch-expected-results-mc.csv # application/src/test/resources/speedtest/travelSearch-expected-results-md.csv # application/src/test/resources/speedtest/travelSearch-expected-results-sr.csv
|
There is a conflict. |
# Conflicts: # application/src/test/java/org/opentripplanner/routing/algorithm/mapping/__snapshots__/ElevationSnapshotTest.snap
| time_ms += (long) Math.ceil(1000.0 * turnDuration); | ||
| weight += preferences.street().turnReluctance() * turnDuration; | ||
| weight += preferences.street().turnReluctance() * modeReluctance * turnDuration; |
There was a problem hiding this comment.
Should we do (turn reluctance + mode reluctance) * turn duration instead? We have had issues in the past the reluctance becomes supra linear. Same for the stairs reluctance.
There was a problem hiding this comment.
Sorry, reluctance is a factor. They can't be added together.
There was a problem hiding this comment.
Sorry I made a mistake so (turn reluctance + mode reluctance - 1) * turn duration would be more correct, but we can discuss this in another meeting.
There was a problem hiding this comment.
This doesn't work mathematically as well. For example, put turn reluctance as 0.5 and mode reluctance as 0.2, it will result in a negative number.
Summary
This change stairs reluctance and turn reluctance to be applied on top of the reluctance for the street mode, rather than replace it.
Issue
None yet, but related to #6662 . It will be more user-friendly to say "avoiding stairs" is compared to walking on flat ground, rather than compared to taking transit.
For turns, the original code was incorrect as the time taken on foot / bike / car in turns were not affected by the car / bike / walk reluctance set, which this PR corrects it.
Unit tests
updated
Documentation
N/A
Changelog
N/A
Bumping the serialization version id
Not needed