Skip to content

Fix combination of flex access and constrained transfer#3726

Merged
t2gran merged 5 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fix_flex_transfers
Nov 9, 2021
Merged

Fix combination of flex access and constrained transfer#3726
t2gran merged 5 commits into
opentripplanner:dev-2.xfrom
entur:otp2_fix_flex_transfers

Conversation

@hannesj

@hannesj hannesj commented Nov 5, 2021

Copy link
Copy Markdown
Contributor

Summary

Currently RAPTOR crashes when combining a flexible access with constrained transfers. This PS fixes that

Unit tests

Added unit test for this feature combination.

Code style

Code style followed

Documentation

Nonen required

Changelog

The changelog file
is generated from the pull-request title, make sure the title describe the feature or issue fixed.
To exclude the PR from the changelog add [changelog skip] in the title.

@hannesj hannesj requested a review from a team as a code owner November 5, 2021 14:15
@t2gran t2gran self-requested a review November 8, 2021 08:52
@t2gran t2gran added this to the 2.1 milestone Nov 8, 2021

@t2gran t2gran 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.

The FLEX access, especially the arrival on-board, create a lot of exception cases in the Raptor routing logic, optimized transfer and path mapping. This bugfix add tho this. I am not so happy about this and think I want to look into a better way of encapsulating this logic. I do NOT think we should do it as part of this PR, but have it in mind when we look at improving the heuristic. We might even drop some of the Raptor variants at some point - making the changes a little easier.

The previous() method is not used in the inner routing loop and the previousTransit is only used when finding constrained transfers, so I am not to worried about the degrade in performance for these changes - even if they do change som of the core logic.

@t2gran t2gran added the !Bug Apply to issues describing a bug and PRs witch fixes it. label Nov 8, 2021
t2gran
t2gran previously approved these changes Nov 8, 2021
@hannesj hannesj added the Entur Test This is currently being tested at Entur label Nov 8, 2021

@leonardehrenfried leonardehrenfried 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.

Looks good. I fixed a tiny typo though.

@hannesj hannesj dismissed stale reviews from leonardehrenfried and t2gran via 45d75f1 November 9, 2021 11:17
…11_GuaranteedTransferWithFlexAccessTest.java

Co-authored-by: Leonard Ehrenfried <mail@leonard.io>
@t2gran t2gran merged commit 484c446 into opentripplanner:dev-2.x Nov 9, 2021
@t2gran t2gran deleted the otp2_fix_flex_transfers branch November 9, 2021 11:33
t2gran pushed a commit that referenced this pull request Nov 9, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants