Skip to content
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

Allow multiple zones in an unscheduled flex trip #5376

Merged

Conversation

leonardehrenfried
Copy link
Member

@leonardehrenfried leonardehrenfried commented Sep 26, 2023

Summary

This allows more than 2 stop times for an unscheduled flex trip, which is legal in GTFS Flex and according to @t2gran also in Netex.

This allows you to have more complex rules about when you're allowed to travel from one zone to another.

In this real world example from Seattle, you are allowed to travel within zone area_1081 but not within zone area_897. You need to board in zone area_1081 to get to area_897.

trip_id stop_id stop_sequence pickup_type drop_off_type start_pickup_drop_off_window end_pickup_drop_off_window
t_5587704_b_78527_tn_0 area_1081 1 2 1 07:00:00 19:00:00
t_5587704_b_78527_tn_0 area_1081 2 1 2 07:00:00 19:00:00
t_5587704_b_78527_tn_0 area_897 3 1 2 07:00:00 19:00:00

Visual examples of the desired result

You are allowed to travel from King County (south) to Snohomish County (North) on the service called "Volunteer Service".

Screenshot from 2023-09-25 20-01-39

However, when you flip the origin/destination you will not be offered this service due to the board/alight restrictions on the trip.
Screenshot from 2023-09-25 18-20-17

Unit tests

Lots added.

I've also refactored the UnscheduledTripTest to use parameterised tests and extracted the integration tests from the unit tests.

Documentation

Javadoc updated.

cc @tsherlockcraig

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (b3f1ae7) 66.58% compared to head (07de657) 66.62%.

Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5376      +/-   ##
=============================================
+ Coverage      66.58%   66.62%   +0.04%     
- Complexity     15318    15336      +18     
=============================================
  Files           1792     1792              
  Lines          69540    69537       -3     
  Branches        7333     7334       +1     
=============================================
+ Hits           46301    46330      +29     
+ Misses         20756    20733      -23     
+ Partials        2483     2474       -9     
Files Coverage Δ
...er/ext/flex/template/FlexAccessEgressTemplate.java 71.42% <ø> (ø)
.../main/java/org/opentripplanner/model/StopTime.java 77.27% <ø> (+7.60%) ⬆️
.../java/org/opentripplanner/ext/flex/FlexRouter.java 91.89% <75.00%> (ø)
...opentripplanner/ext/flex/trip/UnscheduledTrip.java 79.46% <92.85%> (+18.07%) ⬆️
...ipplanner/ext/flex/trip/ScheduledDeviatedTrip.java 55.55% <0.00%> (+1.10%) ⬆️

... and 2 files with indirect coverage changes

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

@leonardehrenfried leonardehrenfried changed the title Allow multiple zones in and unscheduled flex trip Allow multiple zones in an unscheduled flex trip Sep 26, 2023
@t2gran t2gran added this to the 2.5 (next release) milestone Sep 28, 2023
@leonardehrenfried
Copy link
Member Author

@t2gran I had an idea how to make the scope of this PR smaller: how about I allow only one or two zones in an UnscheduledTrip? This would mean that the case of three or more zones would not have to be considered.

@t2gran
Copy link
Member

t2gran commented Oct 11, 2023

We have discussed this issue at Entur today, and I think we can go forward with this PR. This mean that we agree one the business rule:

An unscheduled flex trip may visit/drive from one flex stops(areas/group of stop locations) to any other stop in the pattern without driving through the stops in between. Only the times in the two stops used need to match the path.

@leonardehrenfried Can a trip go from Stop 3 to Stop 2 if the times allow it?

@leonardehrenfried
Copy link
Member Author

We have discussed this issue at Entur today, and I think we can go forward with this PR.

Great - thanks for the feedback!

This mean that we agree one the business rule:

An unscheduled flex trip may visit/drive from one flex stops(areas/group of stop locations) to any other stop in the pattern without driving through the stops in between. Only the times in the two stops used need to match the path.

I will add this rule to the Javadoc.

@leonardehrenfried Can a trip go from Stop 3 to Stop 2 if the times allow it?

My understanding is that it should not be possible but I will write a unit test to make sure. Or do you believe that it should be possible?

@leonardehrenfried
Copy link
Member Author

Actually, there was already a test for that and the code for it is here:

// templates will be generated from the boardingIndex to the end of the trip
final int lastIndexInTrip = stopTimes.length - 1;

@tsherlockcraig
Copy link
Contributor

My understanding is that it should not be possible but I will write a unit test to make sure. Or do you believe that it should be possible?

I also agree that this should not be possible. If travel from 3 to 2 is possible, then there should be another trip where 3 appears before 2 in the stop sequence. The rule still holds that stop_sequence increases during a trip, which I've tried to make explicit with an addition to the GTFS-flex spec here: https://github.com/google/transit/pull/388/files/e359750cb2d4a496cf96bdbb1c6e30a73b3fb59f..2efafbfe2b91e0b99313df2391adb3fbc9121861#diff-3ecf0760eb54b4953728042a1e30586705dc2335807be94faae0de5829cd12a1

@leonardehrenfried leonardehrenfried requested review from t2gran and removed request for vpaturet October 16, 2023 09:32
@leonardehrenfried leonardehrenfried merged commit 9c9addd into opentripplanner:dev-2.x Oct 20, 2023
@leonardehrenfried leonardehrenfried deleted the multiple-area-stops branch October 20, 2023 13:09
t2gran pushed a commit that referenced this pull request Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants