Skip to content

Add cost to maximize wait-time and avoid back-travel in optimize transfers - #3654

Merged
t2gran merged 3 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fixoptimize_transfers
Nov 12, 2021
Merged

t2gran merged 3 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fixoptimize_transfers

Conversation

@t2gran

@t2gran t2gran commented Oct 15, 2021

Copy link
Copy Markdown
Member

Summary

This is an enhancement of the previous PRs on transfers. It try to balance constrained transfers and avoid back-travel. There is detail documentation on who this works in the PR.

Issue

There is no issue for this.

Unit tests

Unit tests are updated.

Code style

Documentation

@t2gran t2gran added !Bug Apply to issues describing a bug and PRs witch fixes it. !Technical Debt Improve code quality, no functional changes. Entur Test This is currently being tested at Entur labels Oct 15, 2021
@t2gran t2gran added this to the 2.1 milestone Oct 15, 2021
@t2gran
t2gran force-pushed the otp2_fixoptimize_transfers branch from d8d745d to 7ce8fe1 Compare October 18, 2021 10:30
@t2gran
t2gran marked this pull request as ready for review October 21, 2021 14:33
@t2gran
t2gran requested a review from a team as a code owner October 21, 2021 14:33
@t2gran
t2gran force-pushed the otp2_fixoptimize_transfers branch from 7ce8fe1 to 35be747 Compare October 21, 2021 14:54
@t2gran
t2gran marked this pull request as draft October 21, 2021 15:04
t2gran added a commit that referenced this pull request Oct 22, 2021
refactor: Some OTP cleanup to prepare for #3654
@t2gran
t2gran marked this pull request as ready for review October 22, 2021 09:52
@t2gran
t2gran marked this pull request as draft October 22, 2021 10:48

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved subject to updating the comment line in change.md.

Comment thread change.md Outdated
@t2gran
t2gran force-pushed the otp2_fixoptimize_transfers branch from ee2279c to fb9e4b5 Compare November 5, 2021 14:14
@t2gran
t2gran marked this pull request as ready for review November 5, 2021 14:15
@t2gran t2gran changed the title Include generalized-cost in optimized-wait-time-cost Add cost to maximize wait-time and avoid back-travel Nov 5, 2021
@t2gran t2gran changed the title Add cost to maximize wait-time and avoid back-travel Add cost to maximize wait-time and avoid back-travel in optimize transfers Nov 5, 2021
@t2gran
t2gran force-pushed the otp2_fixoptimize_transfers branch from fb9e4b5 to 4b9b4c2 Compare November 5, 2021 14:45
Comment thread docs/Configuration.md Outdated
@t2gran
t2gran requested a review from optionsome November 9, 2021 10:35

@hannesj hannesj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one comment on the documentation

@optionsome optionsome left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes looked good and tested with a case that has back-travel in OTP1 but not with these changes in OTP2.

return Math.min(MIN_SAFE_TRANSFER_TIME_LIMIT_UPPER_BOUND, minSafeTransitTime);
return bound(
minSafeTransitTime,
MIN_SAFE_TRANSFER_TIME_LIMIT_LOWER_BOUND,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably needs to be changed once support for defining the GTFS transfers.txt min_transfer_time is added?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if it has anything to do with each other. The minSafeTransitTime is used to add a cost for very short transfers across all possible transfers for a set of trips, while the GTFS min_transfer_time is a restriction for a given pair of stops(& trips). The OptimizeTransferService and the Raptor search must both support the GTFS min_transfer_time - preventing transfers when appropriate.

@t2gran
t2gran merged commit 80a907e into opentripplanner:dev-2.x Nov 12, 2021
@t2gran
t2gran deleted the otp2_fixoptimize_transfers branch November 12, 2021 12:21
t2gran pushed a commit that referenced this pull request Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Bug Apply to issues describing a bug and PRs witch fixes it. Entur Test This is currently being tested at Entur !Technical Debt Improve code quality, no functional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants