Change elevator cost calculation, add `reluctance` for elevators, and deprecate `elevatorHopCost` #7111

VillePihlava · 2025-12-03T13:52:16Z

Summary

Elevator preferences had separate cost fields previously. This is different compared to the reluctance value used in other preferences. Elevators should not be automatically preferred over reluctance-using options, e.g. stairs and escalators. Setting the costs separately is tedious and having some kind of default reluctance seems like a good thing to add. Adding a default reluctance of 2.0 makes the routing a bit better.

The chosen solution

config elevator { reluctance, boardCost, boardSlack, hopTime }
elevator-cost := boardCost + reluctance * elevator-total-time
elevator-total-time := boardSlack + elevator-travel-time
elevator-travel-time := osmTravelTime, or if not set: numberOfHops * hopTime

What this PR does?

Add elevatorReluctance and remove elevatorBoardCost and elevatorHopCost
Now existing configurable variables elevatorBoardTime, elevatorHopTime, and elevatorReluctance
Elevator hop cost change:
- was: amountOfLevelsTraveled * elevatorHopCost
  OR
  travelTime (for specific elevator from OSM)
- is now: elevatorReluctance * amountOfLevelsTraveled * elevatorHopTime
  OR
  elevatorReluctance * travelTime (for specific elevator from OSM)
Elevator board cost change:
- was: elevatorBoardCost
- is now: elevatorReluctance * elevatorBoardTime

Reasoning behind change

Favoring elevators over stairs or escalators can be bad if everyone is routed through elevators. When everyone uses the same elevator, wait times increase. The throughput is not as good as stairs or escalators. However, elevators should not be penalized too much because it should still make sense to use them in some cases. Setting the default value for the elevator reluctance to 2.0 puts it a bit above the default value for escalators (1.5) and equal to the default reluctance value for stairs in walk preferences.

A simpler version of this PR would simply increase the default values for elevatorBoardCost and elevatorHopCost. However, since escalators and stairs use a reluctance value, I think elevators should too. Comparing reluctances is easier than comparing reluctances and constant values.

In my opinion, the board cost/wait cost for transit is different than the board cost for elevators. For elevators, the competition is stairs, sloped streets, and escalators. For transit the competition is other transit, walking, cycling, waiting for transit, and the hassle of changing transit vehicles.

The more relevant metric for elevators is the total time taken by the elevator journey, not an individual board cost (the underlying hop times and board times can still be changed). This total time of the elevator journey then competes with the other options. Having one elevator reluctance value for the total time taken makes setting the ratios between the competition simpler.

Issue

Related to #6829

Unit tests

Changed existing tests because of new way of calculating elevator cost.

Documentation

Router config field added with short description.

VillePihlava · 2025-12-03T13:55:38Z

Is there a transmodel GraphQL API label?

VillePihlava · 2025-12-03T13:58:21Z

This PR would remove the elevatorBoardCost and elevatorHopCost in favor of elevatorReluctance from the transmodel API, which needs to be discussed. A CI check also fails because of this

codecov · 2025-12-03T14:01:01Z

Codecov Report

❌ Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.11%. Comparing base (08cba24) to head (675f450).
⚠️ Report is 156 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...ng/api/request/preference/ElevatorPreferences.java	95.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #7111      +/-   ##
=============================================
+ Coverage      72.06%   72.11%   +0.04%     
- Complexity     20681    20702      +21     
=============================================
  Files           2238     2245       +7     
  Lines          83672    83931     +259     
  Branches        8348     8348              
=============================================
+ Hits           60297    60523     +226     
- Misses         20451    20483      +32     
- Partials        2924     2925       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

miklcct · 2025-12-03T14:41:04Z

We would like to make this configurable client-side with the GTFS API, so that we can add a preference of "avoid elevators", thanks.

VillePihlava · 2025-12-04T14:26:19Z

I updated the description to describe the changes in more detail. If my argument for adding the reluctance value is not supported by other OTP developers I'll close this PR and create another one for changing the default values of elevatorBoardCost and elevatorHopCost. Adding a reluctance value for the hop time alone doesn't make sense in my opinion

t2gran · 2025-12-04T14:37:31Z

I will review the design, and answer the question about the Transmodel API

application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql

t2gran · 2025-12-09T14:51:33Z

My suggestions,

config elevator { reluctance, boardCost, boardSlack }
elevator-travel-time := osmTravelTime, or if not set: numberOfHops * elevatorHopTime
elevator-total-time := boardSlack + elevator-travel-time
Alt A: elevator-cost := boardCost + reluctance * elevator-total-time, or
Alt B: elevator-cost := boardCost + reluctance * elevator-travel-time + waitReluctance * boardSlack

Choosing between alt. A and B depend on if we regard the boardSlack as expected wait-time or some kind, or penalty on the time it takes to use the elevator. The elevator-travel-time calculation is questionable.

I think the above suggestion is inline with the rest of the code. The terms reluctance, slack and so on should have the same meaning all over.

VillePihlava · 2025-12-09T15:08:59Z

My suggestions,

config elevator { reluctance, boardCost, boardSlack }

elevator-travel-time := osmTravelTime, or if not set: numberOfHops * elevatorHopTime

elevator-total-time := boardSlack + elevator-travel-time

Alt A: elevator-cost := boardCost + reluctance * elevator-total-time, or

Alt B: elevator-cost := boardCost + reluctance * elevator-travel-time + waitReluctance + boardSlack

Choosing between alt. A and B depend on if we regard the boardSlack as expected wait-time or some kind, or penalty on the time it takes to use the elevator. The elevator-travel-time calculation is questionable.

I think the above suggestion is inline with the rest of the code. The terms reluctance, slack and so on should have the same meaning all over.

I think you are missing hopTime in the config on this line:

config elevator { reluctance, boardCost, boardSlack }

I think you also meant waitReluctance * boardSlack

Alt B: elevator-cost := boardCost + reluctance * elevator-travel-time + waitReluctance + boardSlack

I'm in favor of option A.

Should we create a separate config object for elevatorpreferences? What I mean is that instead of having elevatorReluctance directly under routingDefaults, having something like the bicycle object in the example, but for elevators:

"routingDefaults": {
    "elevatorBoardTime": 60,
    "bicycle": {
      "boardCost": 120,
      "reluctance": 1.7,
    }
}

optionsome · 2025-12-12T22:29:39Z

I discussed this with @t2gran and came to the conclusion that we should do elevator-cost := boardCost + reluctance * elevator-total-time, because waitReluctance is not meant to be used outside of the transit context.

…= boardCost + reluctance * elevator-total-time'.

VillePihlava · 2025-12-15T15:46:40Z

This is ready for review now

leonardehrenfried · 2025-12-17T10:10:13Z

...ication/src/main/java/org/opentripplanner/apis/transmodel/model/DefaultRouteRequestType.java

+        GraphQLFieldDefinition.newFieldDefinition()
+          .name("elevatorBoardSlack")
+          .description("How long it takes to get on an elevator, on average.")
+          .type(Scalars.GraphQLInt)


Do we have a duration type in the API? I would prefer it rather than the number of seconds (which these presumably are, but not documented.)

All time related fields in this file seem to be using the type Scalars.GraphQLInt. Another option would be to not add this to the API at all, it can be added later on if someone needs it

I removed elevatorBoardSlack from the API and removed the deprecation from elevatorBoardTime

leonardehrenfried · 2025-12-17T10:17:06Z

application/src/main/java/org/opentripplanner/street/model/edge/ElevatorHopEdge.java

    //
    // X   ElevatorHopVertex
    // --- ElevatorHopEdge
+    double time = request.elevator().hopTime().toSeconds() * this.levels;


Please add test cases in ElevatorHopEdge for the various cases:

fixed travel time

travel time from config

...

Sorry, I meant ElevatorHopEdge.

I improved the tests, was it what you meant?

leonardehrenfried

This looks already quite good but I have two comments.

Add elevatorReluctance and remove elevatorBoardCost and elevatorHopCost.

ac9a070

VillePihlava requested a review from a team as a code owner December 3, 2025 13:52

VillePihlava added !Improvement A functional improvement or micro feature +NeTEx This issue is related to the Netex model/import. +Config Change This PR might require the configuration to be updated. and removed +NeTEx This issue is related to the Netex model/import. labels Dec 3, 2025

Fix elevatorReluctance type from int to float.

a81174c

optionsome reviewed Dec 9, 2025

View reviewed changes

application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql Show resolved Hide resolved

optionsome assigned tkalvas, leonardehrenfried and t2gran Dec 9, 2025

Deprecate fields instead of removing them.

66aea01

Change elevator cost calculation to use the formula: 'elevator-cost :…

48a03c8

…= boardCost + reluctance * elevator-total-time'.

VillePihlava changed the title ~~Add elevatorReluctance and remove elevatorBoardCost and elevatorHopCost~~ Change elevator cost calculation, add reluctance for elevators, and deprecate elevatorHopCost Dec 15, 2025

Rename boardTime to boardSlack.

0a1bc95

Fix typo.

4f3cb89

optionsome unassigned t2gran Dec 16, 2025

leonardehrenfried reviewed Dec 17, 2025

View reviewed changes

leonardehrenfried requested changes Dec 17, 2025

View reviewed changes

optionsome requested a review from tkalvas December 18, 2025 13:53

VillePihlava added 2 commits December 19, 2025 11:20

Improve ElevatorHopEdge tests.

68a822d

Remove elevatorBoardSlack from transmodel API.

675f450

VillePihlava requested a review from leonardehrenfried December 19, 2025 09:33

optionsome added the +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR label Dec 19, 2025

Change elevator cost calculation, add reluctance for elevators, and deprecate elevatorHopCost #7111

Are you sure you want to change the base?

Change elevator cost calculation, add reluctance for elevators, and deprecate elevatorHopCost #7111

Conversation

VillePihlava commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The chosen solution

What this PR does?

Reasoning behind change

Issue

Unit tests

Documentation

Uh oh!

VillePihlava commented Dec 3, 2025

Uh oh!

VillePihlava commented Dec 3, 2025

Uh oh!

codecov bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

miklcct commented Dec 3, 2025

Uh oh!

VillePihlava commented Dec 4, 2025

Uh oh!

t2gran commented Dec 4, 2025

Uh oh!

Uh oh!

t2gran commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VillePihlava commented Dec 9, 2025

Uh oh!

optionsome commented Dec 12, 2025

Uh oh!

VillePihlava commented Dec 15, 2025

Uh oh!

leonardehrenfried Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

VillePihlava Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

VillePihlava Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

VillePihlava Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Change elevator cost calculation, add `reluctance` for elevators, and deprecate `elevatorHopCost` #7111

Change elevator cost calculation, add `reluctance` for elevators, and deprecate `elevatorHopCost` #7111

VillePihlava commented Dec 3, 2025 •

edited

Loading

codecov bot commented Dec 3, 2025 •

edited

Loading

t2gran commented Dec 9, 2025 •

edited

Loading

leonardehrenfried Dec 17, 2025 •

edited

Loading