Add flex stop to TripTimes, return geometries in GraphQL API #3757

leonardehrenfried · 2021-11-25T11:21:25Z

Summary

As discussed in previous meetings this does the following:

changes the internal data model to work with StopLocation instead of Stop
thereby adding them to the trip times where they will automatically show up in the APIs
adding GeoJSON and polyline encodings of the flex stop geometries

If you want to know what the GraphQL output looks like, check this out: https://gist.github.com/leonardehrenfried/d2b3abb6bd14a2237bb319ff440f7c89

It wasn't very easy to exclude the stops from the Raptor code, so I disallowed boarding and alighting from them in the Raptor code. This adds another check for each stop in the Raptor code so it has a potential performance impact. I'm running the speed tests right now to see how bad it is.

@hannesj has made a really good suggestion: set the board/alight permissions during instantiation of StopPattern so adding these stops doesn't have a runtime cost.

Issue

#3756

Unit tests

Tests were added.

Code style

Yes.

Documentation

I've added lots of JavaDoc, particularly for the tests.

optionsome · 2021-11-26T11:33:56Z

Tried to build this with http://oregon-gtfs.com/gtfs_data/lincolncounty-or-us/lincolncounty-or-us--flex-v2.zip and it crashed

13:31:42.028 ERROR (OTPMain.java:59) An uncaught error occurred inside OTP: Could not interpolate arrival/departure time on stop 0 (missing final stop time) on trip <Trip lincolncounty-or-us:t_299275_b_26748_tn_0>
java.lang.RuntimeException: Could not interpolate arrival/departure time on stop 0 (missing final stop time) on trip <Trip lincolncounty-or-us:t_299275_b_26748_tn_0>
	at org.opentripplanner.gtfs.RepairStopTimesForEachTripOperation.interpolateStopTimes(RepairStopTimesForEachTripOperation.java:281)
	at org.opentripplanner.gtfs.RepairStopTimesForEachTripOperation.run(RepairStopTimesForEachTripOperation.java:69)
	at org.opentripplanner.graph_builder.module.GtfsModule.repairStopTimesForEachTrip(GtfsModule.java:169)
	at org.opentripplanner.graph_builder.module.GtfsModule.buildGraph(GtfsModule.java:129)
	at org.opentripplanner.graph_builder.GraphBuilder.run(GraphBuilder.java:80)
	at org.opentripplanner.standalone.OTPMain.startOTPServer(OTPMain.java:136)
	at org.opentripplanner.standalone.OTPMain.main(OTPMain.java:52)

leonardehrenfried · 2021-11-26T11:48:50Z

Does this work with the latest dev-2.x version?

optionsome · 2021-11-26T11:54:00Z

At least it builds, haven't tested otherwise

leonardehrenfried · 2021-11-26T12:02:12Z

I think I know what the problem is.

The trip in question has the following stop times:

stop_times.txt
881:t_299275_b_26748_tn_0,,,radius_1207_s_16069_s_16024,1,,0,0,0,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
882:t_299275_b_26748_tn_0,,,radius_1207_s_16024_s_16025,2,,0,0,775.596266212284,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
883:t_299275_b_26748_tn_0,,,radius_1207_s_16025_s_16026,3,,0,0,1807.22190535612,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
884:t_299275_b_26748_tn_0,,,radius_1207_s_16026_s_2483495,4,,0,0,2766.28482550726,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
885:t_299275_b_26748_tn_0,,,radius_1207_s_2483495_s_29681,5,,0,0,5416.75044306174,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
886:t_299275_b_26748_tn_0,,,radius_1207_s_29681_s_16032,6,,0,0,6436.45201007476,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
887:t_299275_b_26748_tn_0,,,radius_1207_s_16032_s_2438933,7,,0,0,7576.71452995217,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
888:t_299275_b_26748_tn_0,,,radius_1207_s_2438933_s_16031,8,,0,0,8240.84195654157,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
889:t_299275_b_26748_tn_0,,,radius_1207_s_16031_s_16030,9,,0,0,8527.62288862872,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
890:t_299275_b_26748_tn_0,,,radius_1207_s_16030_s_16033,10,,0,0,8932.84679637186,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
891:t_299275_b_26748_tn_0,,,radius_1207_s_16033_s_16034,11,,0,0,9165.13404331625,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
892:t_299275_b_26748_tn_0,,,radius_1207_s_16034_s_15889,12,,0,0,9628.59060260394,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
893:t_299275_b_26748_tn_0,,,radius_1207_s_15889_s_16039,13,,0,0,10510.9386855591,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
894:t_299275_b_26748_tn_0,,,radius_1207_s_16039_s_16040,14,,0,0,11686.4154079233,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
895:t_299275_b_26748_tn_0,,,radius_1207_s_16040_s_16042,15,,0,0,12047.7835753043,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
896:t_299275_b_26748_tn_0,,,radius_1207_s_16042_s_16043,16,,0,0,12324.4604601439,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
897:t_299275_b_26748_tn_0,,,radius_1207_s_16043_s_16044,17,,0,0,12526.5458089135,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
898:t_299275_b_26748_tn_0,,,radius_1207_s_16044_s_16045,18,,0,0,12886.5424727922,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
899:t_299275_b_26748_tn_0,,,radius_1207_s_16045_s_15942,19,,0,0,13106.2063391676,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
900:t_299275_b_26748_tn_0,,,radius_1207_s_15942_s_16048,20,,0,0,15282.8820371869,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
901:t_299275_b_26748_tn_0,,,radius_1207_s_16048_s_2438934,21,,2,3,17060.1720314054,0,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
902:t_299275_b_26748_tn_0,,,radius_1207_s_2438934_s_16049,22,,0,0,18604.0056042532,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
903:t_299275_b_26748_tn_0,,,radius_1207_s_16049_s_16050,23,,0,0,19098.7301834617,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
904:t_299275_b_26748_tn_0,,,radius_1207_s_16050_s_16051,24,,0,0,19388.6880759922,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
905:t_299275_b_26748_tn_0,,,radius_1207_s_16051_s_16052,25,,0,0,20425.6382579853,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
906:t_299275_b_26748_tn_0,,,radius_1207_s_16052_s_16053,26,,0,0,20752.5347065814,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
907:t_299275_b_26748_tn_0,,,radius_1207_s_16053_s_833259,27,,0,0,21230.9536762786,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
908:t_299275_b_26748_tn_0,,,radius_1207_s_833259_s_16275,28,,0,0,23127.0336395412,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
909:t_299275_b_26748_tn_0,,,radius_1207_s_16275_s_16055,29,,0,0,23551.6916242021,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
910:t_299275_b_26748_tn_0,,,radius_1207_s_16055_s_16057,30,,0,0,23808.8722434897,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
911:t_299275_b_26748_tn_0,,,radius_1207_s_16057_s_16058,31,,0,0,24487.418244042,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
912:t_299275_b_26748_tn_0,,,radius_1207_s_16058_s_16059,32,,0,0,24680.6102514857,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
913:t_299275_b_26748_tn_0,,,radius_1207_s_16059_s_16060,33,,0,0,25614.1591340582,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
914:t_299275_b_26748_tn_0,,,radius_1207_s_16060_s_16061,34,,0,0,26256.131709164,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
915:t_299275_b_26748_tn_0,,,radius_1207_s_16061_s_16062,35,,0,0,27180.643659028,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
916:t_299275_b_26748_tn_0,,,radius_1207_s_16062_s_16063,36,,0,0,27686.863698439,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
917:t_299275_b_26748_tn_0,,,radius_1207_s_16063_s_15890,37,,0,0,28259.1890347633,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
918:t_299275_b_26748_tn_0,,,radius_1207_s_15890_s_2438932,38,,0,0,28886.6228332024,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
919:t_299275_b_26748_tn_0,,,radius_1207_s_2438932_s_16065,39,,2,3,29619.901749005,0,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
920:t_299275_b_26748_tn_0,,,radius_1207_s_16065_s_2483495,40,,0,0,30254.2525223819,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
921:t_299275_b_26748_tn_0,,,radius_1207_s_2483495_s_16066,41,,2,3,30838.8763853997,0,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
922:t_299275_b_26748_tn_0,,,radius_1207_s_16066_s_16068,42,,0,0,32312.2465968761,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
923:t_299275_b_26748_tn_0,,,radius_1207_s_16068_s_16069,43,,0,0,34898.4147967777,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00
924:t_299275_b_26748_tn_0,,,radius_1207_s_16069_s_16069,44,,0,0,36360.0641114,1,,,,,2,3,booking_route_491,booking_route_491,,,1,00:03:00,1,00:05:00

I has continuous pick up/drop off set to 2 on all stop_times. The code checks this here

OpenTripPlanner/src/ext/java/org/opentripplanner/ext/flex/trip/UnscheduledTrip.java

Line 46 in 63a1182

    
           stopTime.getFlexContinuousDropOff() == NONE.getGtfsCode() && stopTime.getFlexContinuousPickup() == NONE.getGtfsCode();

and thinks it's not a flex trip.

@hannesj Do you know why the check for the continuous pick up/drop off happens?

leonardehrenfried · 2021-11-26T12:58:59Z

It's presumably because of this:

OpenTripPlanner/src/ext/java/org/opentripplanner/ext/flex/FlexTripsMapper.java

Line 42 in a4238b9

// result.add(new ContinuousPickupDropOffTrip(trip, stopTimes));

This functionality doesn't exist anymore, isn't it @hannesj?

leonardehrenfried · 2021-11-26T13:31:09Z

I've tested it locally and if i removed the check for continuous pick up it works as expected.

leonardehrenfried · 2021-11-26T13:40:58Z

But I will wait for a comment by @hannesj before I commit the deletion.

hannesj · 2021-11-26T13:51:24Z

This functionality doesn't exist anymore, isn't it @hannesj?

Yet. There is a draft implementation in #3457

I've tested it locally and if i removed the check for continuous pick up it works as expected.

I'm a bit unsure what that combination of flags would mean? Maybe a suggestion to the GTFS-Flex specification that continuous stops can't be defined for flex areas/groups

leonardehrenfried · 2021-11-26T17:04:21Z

I'm also puzzled what continuous pick up would mean in this case. Doesn't the flex area already imply that?

On the other hand it's probably a permitted combination.

What do you think of keeping the code as is and before doing the interpolation we check if the stop times contain a FlexStopLocation?

leonardehrenfried · 2021-11-26T22:26:39Z

I've pushed a simple fix that excludes any trip with flex stops from the interpolation process. This restores the previous behaviour where those trips would not be recognised as flex trips.

I've also added a test that the GTFS files can be parsed nevertheless.

After some thinking I believe the continuous pick up field is there to stay compatible with the flex v1 spec.

leonardehrenfried · 2021-12-01T10:45:19Z

BTW, @optionsome continuous pickup for flex zones seems to be considered an invalid combination.

If you have another opinion, you can comment on the Flex spec issue: MobilityData/gtfs-flex#70

src/main/java/org/opentripplanner/gtfs/RepairStopTimesForEachTripOperation.java

src/ext/resources/legacygraphqlapi/schema.graphqls

hannesj · 2021-12-02T17:30:35Z

src/main/java/org/opentripplanner/graph_builder/module/geometry/GeometryAndBlockProcessor.java

@@ -374,7 +375,7 @@ private boolean hasShapeDist(FeedScopedId shapeId, List<StopTime> stopTimes) {
                    continue;
                }
                double distance = segment.distance(coord);
-                if (distance < maxStopToShapeSnapDistance) {
+                if (distance < maxStopToShapeSnapDistance || isFlexTrip) {


Should we check if either of the stops is a flex stop instead?

I'm checking whether the trip contains any flex stops. Do you think that this check is too broad?

Yes, we can just check if the variable stop here is an actual stop, and not a flex one

hannesj · 2021-12-02T17:39:36Z

I'll run some tests using Netex flex routes, and approve if it works ok.

hannesj · 2021-12-02T17:46:23Z

The casts in here should be fixed

https://github.com/opentripplanner/OpenTripPlanner/blob/dev-2.x/src/ext/java/org/opentripplanner/ext/transmodelapi/model/stop/QuayType.java

optionsome · 2021-12-02T20:50:57Z

src/ext/java/org/opentripplanner/ext/legacygraphqlapi/datafetchers/LegacyGraphQLStopImpl.java

@@ -132,6 +133,11 @@
    return environment -> null;
  }

+  @Override
+  public DataFetcher<Object> geometries() {
+    return environment -> getValue(environment, StopLocation::getGeometry, station -> null);


Should stations also have geometries so the same method could be used for stations as for stops in clients to draw them etc?

I implemented it as a geometry collection which contains the point of the station and a convex hull of all child stops.

…p/dropoff

leonardehrenfried · 2021-12-10T13:01:49Z

@hannesj I corrected the transfer priority handling for flex stops and could successfully start the Norwegian graph and send queries.

hannesj · 2021-12-13T14:01:33Z

I tested this with the Norwegian graph, and it seems to work properly. However i noticed that the unscheduled stoptimes had been broken for some time. I have fixed those in #3782

Support and change in GTFS behaviour were in opentripplanner#3757

leonardehrenfried requested a review from a team as a code owner November 25, 2021 11:21

leonardehrenfried force-pushed the flex-geometries branch 2 times, most recently from bddcc2a to 130c3f2 Compare November 25, 2021 11:34

hannesj requested review from hannesj and optionsome November 25, 2021 14:08

leonardehrenfried added the IBI test label Nov 25, 2021

t2gran added this to the 2.1 milestone Nov 30, 2021

leonardehrenfried force-pushed the flex-geometries branch 4 times, most recently from 511377e to 08a4522 Compare December 1, 2021 09:59

hannesj reviewed Dec 2, 2021

View reviewed changes

optionsome reviewed Dec 2, 2021

View reviewed changes

leonardehrenfried force-pushed the flex-geometries branch 2 times, most recently from 0e20d0a to d7cc4dc Compare December 7, 2021 21:11

This was referenced Dec 8, 2021

Add complete flex integration test [changelog skip] #3763

Closed

Flexible trips in the index API #3293

Closed

leonardehrenfried added 15 commits December 10, 2021 12:51

Remove redundant filter

ee499a7

Compute geometry for Station, too

8675f81

Initialize stations with coordinates

fdb1eae

Update snapshot expectations

0d8a7a0

Rename polylines to googleEncoded

2427a6c

Add issue when building encountering flex feeds with continuous picku…

1ac23ea

…p/dropoff

Add complete flex integration test

f66c43a

Add test with two transfers

caa0ef6

Clean up tests

8594e77

Actually recompute geometries when adding child stations

b75c031

Update snapshot test

8f37599

Update documentation

223d49c

Return ID if flex stop doesn't have name

ec233ff

Remove flex stops when flex is disabled

a9580aa

Add default implementation of getPriority()

69a219c

leonardehrenfried force-pushed the flex-geometries branch from 9ecb4d5 to 69a219c Compare December 10, 2021 11:52

leonardehrenfried mentioned this pull request Dec 10, 2021

Norway: Travel search from Oslo to Haugesund fails #3777

Closed

hannesj approved these changes Dec 13, 2021

View reviewed changes

optionsome approved these changes Dec 13, 2021

View reviewed changes

leonardehrenfried merged commit 2ec1659 into opentripplanner:dev-2.x Dec 13, 2021

t2gran pushed a commit that referenced this pull request Dec 13, 2021

Add changelog entry for #3757 [ci skip]

987388f

miles-grant-ibigroup deleted the flex-geometries branch December 14, 2021 12:26

hannesj mentioned this pull request Jan 24, 2022

Not allowed transfers and support for GTFS transfer points #3792

Merged

leonardehrenfried added IBI Developed by or important for IBI Group and removed IBI test labels Jun 21, 2022

hannesj added a commit to entur/OpenTripPlanner that referenced this pull request Oct 3, 2022

Remove filtering of flex patterns in NeTEX import

41e6a57

Support and change in GTFS behaviour were in opentripplanner#3757

hannesj mentioned this pull request Oct 3, 2022

Remove filtering of flex patterns in NeTEX import #4493

Merged

hannesj mentioned this pull request Oct 21, 2022

Do not allow boarding/alighting at GroupStops in RAPTOR #4534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flex stop to TripTimes, return geometries in GraphQL API #3757

Add flex stop to TripTimes, return geometries in GraphQL API #3757

leonardehrenfried commented Nov 25, 2021 •

edited

Loading

optionsome commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

optionsome commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

hannesj commented Nov 26, 2021 •

edited

Loading

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Dec 1, 2021

hannesj Dec 2, 2021

leonardehrenfried Dec 3, 2021

hannesj Dec 3, 2021

hannesj commented Dec 2, 2021

hannesj commented Dec 2, 2021

optionsome Dec 2, 2021

leonardehrenfried Dec 3, 2021

leonardehrenfried commented Dec 10, 2021

hannesj commented Dec 13, 2021

Add flex stop to TripTimes, return geometries in GraphQL API #3757

Add flex stop to TripTimes, return geometries in GraphQL API #3757

Conversation

leonardehrenfried commented Nov 25, 2021 • edited Loading

Summary

Issue

Unit tests

Code style

Documentation

optionsome commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

optionsome commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

hannesj commented Nov 26, 2021 • edited Loading

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Nov 26, 2021

leonardehrenfried commented Dec 1, 2021

hannesj Dec 2, 2021

Choose a reason for hiding this comment

leonardehrenfried Dec 3, 2021

Choose a reason for hiding this comment

hannesj Dec 3, 2021

Choose a reason for hiding this comment

hannesj commented Dec 2, 2021

hannesj commented Dec 2, 2021

optionsome Dec 2, 2021

Choose a reason for hiding this comment

leonardehrenfried Dec 3, 2021

Choose a reason for hiding this comment

leonardehrenfried commented Dec 10, 2021

hannesj commented Dec 13, 2021

leonardehrenfried commented Nov 25, 2021 •

edited

Loading

hannesj commented Nov 26, 2021 •

edited

Loading