diff --git a/app/services/geometry.rb b/app/services/geometry.rb index a8651b8fa..02ec4c04d 100644 --- a/app/services/geometry.rb +++ b/app/services/geometry.rb @@ -75,94 +75,19 @@ def self.stop_after_geometry(stop_as_spherical, stop_as_cartesian, line_geometry line_geometry_as_cartesian.after?(stop_as_cartesian) || OutlierStop.outlier_stop_from_precomputed_geometries(stop_as_spherical, stop_as_cartesian, line_geometry_as_cartesian) end - def self.index_of_closest_match_line_segment(locators, s, e, point) - # the method is going forward along the line's direction to find the closest match. - - closest_point_candidates = locators[s..e].map{ |loc| loc.interpolate_point(Stop::GEOFACTORY) } - closest_point_and_dist = closest_point_candidates.map{ |closest_point| - [closest_point, closest_point.distance(point)] - }.detect { |closest_point_and_dist| closest_point_and_dist[1] < FIRST_MATCH_THRESHOLD } - - # Since the first match is within FIRST_MATCH_THRESHOLD, it might not be the best (closest) - # within the search range s through e. - # So here we're walking up the line until we can't find a closer match - the next local minimum. - unless closest_point_and_dist.nil? - dist = closest_point_and_dist[1] - i = closest_point_candidates.index(closest_point_and_dist[0]) - unless i == locators[s..e].size - 1 - next_locators = locators[s..e][i+1..-1].reject{|loc| loc.segment.single_point? } - closer_match = nil - next_locators.each_with_index do |loc, j| - next_seg_dist = loc.interpolate_point(Stop::GEOFACTORY).distance(point) - if next_seg_dist <= dist - closer_match = j - dist = next_seg_dist - else - break - end - end - i = locators[s..e].index(next_locators[closer_match]) unless closer_match.nil? - end - return s + i - end - - # If no match is found within the threshold, just take closest match within the search range. - self.index_of_line_segment_with_nearest_point(locators, s, e) - end - def self.index_of_line_segment_with_nearest_point(locators, start, stop) - # finding the global closest within the start and stop range distances_from_segs = locators[start..stop].map(&:distance_from_segment) start + distances_from_segs.index(distances_from_segs.min) end - def self.fallback_distances(rsp, stops=nil) - rsp.stop_distances = [0.0] - total_distance = 0.0 - stops = rsp.stop_pattern.map {|onestop_id| Stop.find_by_onestop_id!(onestop_id) } if stops.nil? - stops.each_cons(2) do |stop1, stop2| - total_distance += stop1[:geometry].distance(stop2[:geometry]) - rsp.stop_distances << total_distance - end - rsp.stop_distances.map!{ |distance| distance.round(DISTANCE_PRECISION) } - end - - def self.gtfs_shape_dist_traveled(rsp, stop_times, tl_stops, shape_distances_traveled) - # assumes stop times and shapes BOTH have shape_dist_traveled, and they're in the same units - # assumes the line geometry is not generated, and shape_points equals the rsp geometry. - rsp.stop_distances = [] - search_and_seg_index = 0 - stop_times.each_with_index do |st, i| - stop_onestop_id = rsp.stop_pattern[i] - - if st.shape_dist_traveled.to_f < shape_distances_traveled[0] - rsp.stop_distances << 0.0 - elsif st.shape_dist_traveled.to_f > shape_distances_traveled[-1] - rsp.stop_distances << rsp[:geometry].length - else - # Find segment along shape points where stop shape_dist_traveled is between the two shape points' shape_dist_traveled - # need to account for stops matching to same segment - j = -1 - dist1, dist2 = shape_distances_traveled[search_and_seg_index..-1].each_cons(2).detect do |d1, d2| - j += 1 - st.shape_dist_traveled.to_f >= d1 && st.shape_dist_traveled.to_f <= d2 - end - - search_and_seg_index = search_and_seg_index + j - - if dist1.nil? || dist2.nil? - raise StandardError.new("Problem finding stop distance for Stop #{stop_onestop_id}, number #{i + 1} of RSP #{rsp.onestop_id} using shape_dist_traveled") - else - route_line_as_cartesian = self.cartesian_cast(rsp[:geometry]) - stop = tl_stops[i] - locators = route_line_as_cartesian.locators(self.cartesian_cast(stop[:geometry])) - seg_dist = st.shape_dist_traveled.to_f - shape_distances_traveled[search_and_seg_index] - point_on_line = locators[search_and_seg_index].interpolate_point(RGeo::Cartesian::Factory.new(srid: 4326), seg_dist=seg_dist) - rsp.stop_distances << LineString.distance_along_line_to_nearest_point(route_line_as_cartesian, point_on_line, search_and_seg_index) - end - end + def self.index_of_line_segment_with_nearest_point_next_stop(route_line_as_cartesian, tl_stops, test_stop_index, start_seg_index, num_segments) + if (test_stop_index <= tl_stops.size - 1) + next_stop_as_cartesian = self.cartesian_cast(tl_stops[test_stop_index][:geometry]) + next_stop_locators = route_line_as_cartesian.locators(next_stop_as_cartesian) + return self.index_of_line_segment_with_nearest_point(next_stop_locators, start_seg_index, num_segments - 1) + else + return num_segments - 1 end - rsp.stop_distances.map!{ |distance| distance.round(DISTANCE_PRECISION) } end def self.calculate_distances(rsp, stops=nil) @@ -181,67 +106,45 @@ def self.calculate_distances(rsp, stops=nil) b = 0 c = num_segments - 1 last_stop_after_geom = self.stop_after_geometry(stops[-1][:geometry], self.cartesian_cast(stops[-1][:geometry]), route_line_as_cartesian) - previous_stop_before_geom = false + stop_matched_inside_line = false stops.each_index do |i| current_stop = stops[i] current_stop_as_spherical = current_stop[:geometry] current_stop_as_cartesian = self.cartesian_cast(current_stop_as_spherical) if i == 0 && self.stop_before_geometry(current_stop_as_spherical, current_stop_as_cartesian, route_line_as_cartesian) - previous_stop_before_geom = true rsp.stop_distances << 0.0 elsif i == stops.size - 1 && last_stop_after_geom rsp.stop_distances << rsp[:geometry].length else - if (i + 1 <= stops.size - 1) - next_stop = stops[i+1] - next_stop_as_cartesian = self.cartesian_cast(next_stop[:geometry]) - next_stop_locators = route_line_as_cartesian.locators(next_stop_as_cartesian) - c = self.index_of_line_segment_with_nearest_point(next_stop_locators, a, num_segments-1) - else - c = num_segments - 1 - end - + c = self.index_of_line_segment_with_nearest_point_next_stop(route_line_as_cartesian, stops, i+1, a, num_segments) locators = route_line_as_cartesian.locators(current_stop_as_cartesian) - b = self.index_of_closest_match_line_segment(locators, a, c, current_stop_as_cartesian) + b = self.index_of_line_segment_with_nearest_point(locators, a, c) nearest_point = LineString.nearest_point_on_line(locators, b) # The next stop's match may be too early and restrictive, so allow more segment possibilities if LineString.distance_to_nearest_point_on_line(current_stop_as_spherical, nearest_point) > FIRST_MATCH_THRESHOLD - if (i + 2 <= stops.size - 1) - next_stop = stops[i+2] - next_stop_as_cartesian = self.cartesian_cast(next_stop[:geometry]) - next_stop_locators = route_line_as_cartesian.locators(next_stop_as_cartesian) - c = self.index_of_line_segment_with_nearest_point(next_stop_locators, a, num_segments-1) - else - c = num_segments - 1 - end - b = self.index_of_closest_match_line_segment(locators, a, c, current_stop_as_cartesian) + c = self.index_of_line_segment_with_nearest_point_next_stop(route_line_as_cartesian, stops, i+2, a, num_segments) + b = self.index_of_line_segment_with_nearest_point(locators, a, c) nearest_point = LineString.nearest_point_on_line(locators, b) end distance = LineString.distance_along_line_to_nearest_point(route_line_as_cartesian, nearest_point, b) if (i!=0) - if self.stop_before_geometry(current_stop_as_spherical, current_stop_as_cartesian, route_line_as_cartesian) && previous_stop_before_geom - previous_stop_before_geom = true - else - equivalent_stop = stops[i].onestop_id.eql?(stops[i-1].onestop_id) || stops[i][:geometry].eql?(stops[i-1][:geometry]) - if !equivalent_stop && !previous_stop_before_geom - # this can happen if this stop matches to the same segment as the previous - if (distance <= rsp.stop_distances[i-1]) - a += 1 - if (a == num_segments - 1) - distance = rsp[:geometry].length - elsif (a > c) - # we should leave the faulty distance here (unless the interpolation tries to correct it) - # because something might be wrong with the RouteStopPattern. - else - b = self.index_of_closest_match_line_segment(locators, a, c, current_stop_as_cartesian) - nearest_point = LineString.nearest_point_on_line(locators, b) - distance = LineString.distance_along_line_to_nearest_point(route_line_as_cartesian, nearest_point, b) - end + equivalent_stop = stops[i].onestop_id.eql?(stops[i-1].onestop_id) || stops[i][:geometry].eql?(stops[i-1][:geometry]) + if !equivalent_stop + # this can happen if this stop matches to the same segment as the previous + if (distance <= rsp.stop_distances[i-1]) && stop_matched_inside_line + a += 1 + if (a == num_segments - 1) + distance = rsp[:geometry].length + elsif (a > c) + # Something might be wrong with the RouteStopPattern. + else + b = self.index_of_line_segment_with_nearest_point(locators, a, c) + nearest_point = LineString.nearest_point_on_line(locators, b) + distance = LineString.distance_along_line_to_nearest_point(route_line_as_cartesian, nearest_point, b) end end - previous_stop_before_geom = false end end @@ -258,10 +161,60 @@ def self.calculate_distances(rsp, stops=nil) else rsp.stop_distances << distance end + stop_matched_inside_line = true if !nearest_point.eql?(route_line_as_cartesian.points.first) a = b end end # end stop pattern loop rsp.stop_distances.map!{ |distance| distance.round(DISTANCE_PRECISION) } end + + def self.fallback_distances(rsp, stops=nil) + rsp.stop_distances = [0.0] + total_distance = 0.0 + stops = rsp.stop_pattern.map {|onestop_id| Stop.find_by_onestop_id!(onestop_id) } if stops.nil? + stops.each_cons(2) do |stop1, stop2| + total_distance += stop1[:geometry].distance(stop2[:geometry]) + rsp.stop_distances << total_distance + end + rsp.stop_distances.map!{ |distance| distance.round(DISTANCE_PRECISION) } + end + + def self.gtfs_shape_dist_traveled(rsp, stop_times, tl_stops, shape_distances_traveled) + # assumes stop times and shapes BOTH have shape_dist_traveled, and they're in the same units + # assumes the line geometry is not generated, and shape_points equals the rsp geometry. + rsp.stop_distances = [] + search_and_seg_index = 0 + stop_times.each_with_index do |st, i| + stop_onestop_id = rsp.stop_pattern[i] + + if st.shape_dist_traveled.to_f < shape_distances_traveled[0] + rsp.stop_distances << 0.0 + elsif st.shape_dist_traveled.to_f > shape_distances_traveled[-1] + rsp.stop_distances << rsp[:geometry].length + else + # Find segment along shape points where stop shape_dist_traveled is between the two shape points' shape_dist_traveled + # need to account for stops matching to same segment + j = -1 + dist1, dist2 = shape_distances_traveled[search_and_seg_index..-1].each_cons(2).detect do |d1, d2| + j += 1 + st.shape_dist_traveled.to_f >= d1 && st.shape_dist_traveled.to_f <= d2 + end + + search_and_seg_index = search_and_seg_index + j + + if dist1.nil? || dist2.nil? + raise StandardError.new("Problem finding stop distance for Stop #{stop_onestop_id}, number #{i + 1} of RSP #{rsp.onestop_id} using shape_dist_traveled") + else + route_line_as_cartesian = self.cartesian_cast(rsp[:geometry]) + stop = tl_stops[i] + locators = route_line_as_cartesian.locators(self.cartesian_cast(stop[:geometry])) + seg_dist = st.shape_dist_traveled.to_f - shape_distances_traveled[search_and_seg_index] + point_on_line = locators[search_and_seg_index].interpolate_point(RGeo::Cartesian::Factory.new(srid: 4326), seg_dist=seg_dist) + rsp.stop_distances << LineString.distance_along_line_to_nearest_point(route_line_as_cartesian, point_on_line, search_and_seg_index) + end + end + end + rsp.stop_distances.map!{ |distance| distance.round(DISTANCE_PRECISION) } + end end end diff --git a/app/services/gtfs_graph.rb b/app/services/gtfs_graph.rb index 85dd42129..a7d81d0aa 100644 --- a/app/services/gtfs_graph.rb +++ b/app/services/gtfs_graph.rb @@ -193,8 +193,7 @@ def calculate_rsp_distances(rsp, stops, shape_distances_traveled, stop_times) # edited rsps will probably have a shape Geometry::DistanceCalculation.fallback_distances(rsp, stops=stops) elsif (rsp.stop_distances.compact.empty? || rsp.issues.map(&:issue_type).include?(:distance_calculation_inaccurate)) - # avoid writing over stop distances computed with shape_dist_traveled, or already computed somehow - - # unless if rsps have inaccurate stop distances, we'll allow a recomputation if there's a fix in place. + # avoid writing over stop distances that have been computed already and have no issues. Geometry::DistanceCalculation.calculate_distances(rsp, stops=stops) end rescue => e diff --git a/spec/factories/feed_factory.rb b/spec/factories/feed_factory.rb index 3196510ef..d98167ff0 100644 --- a/spec/factories/feed_factory.rb +++ b/spec/factories/feed_factory.rb @@ -274,6 +274,25 @@ end end + factory :feed_mbta, parent: :feed, class: Feed do + onestop_id 'f-drt-mbta' + url 'http://www.mbta.com/uploadedfiles/MBTA_GTFS.zip' + version 1 + after :create do |feed, evaluator| + operator = create( + :operator, + name: 'MBTA', + onestop_id: 'o-drt-mbta', + timezone: 'America/New_York', + website: 'http://www.google.com', + ) + feed.operators_in_feed.create( + operator: operator, + gtfs_agency_id: '1' + ) + end + end + factory :feed_grand_river, parent: :feed, class: Feed do onestop_id 'f-dpwz-grandrivertransit' url 'http://www.regionofwaterloo.ca/opendatadownloads/GRT_GTFS.zip' diff --git a/spec/factories/feed_version_factory.rb b/spec/factories/feed_version_factory.rb index 0c44292e3..fd50866d5 100644 --- a/spec/factories/feed_version_factory.rb +++ b/spec/factories/feed_version_factory.rb @@ -136,6 +136,16 @@ association :feed, factory: :feed_nj_path end + factory :feed_version_mbta_33884627 do + file { File.open(Rails.root.join('spec/support/example_gtfs_archives/mbta_trip_33884627.zip')) } + association :feed, factory: :feed_mbta + end + + factory :feed_version_marta_trip_5449755 do + file { File.open(Rails.root.join('spec/support/example_gtfs_archives/marta_trip_5449755.zip')) } + association :feed, factory: :feed_marta + end + factory :feed_version_marta do file { File.open(Rails.root.join('spec/support/example_gtfs_archives/marta-trip-5453552.zip')) } association :feed, factory: :feed_marta diff --git a/spec/services/geometry_spec.rb b/spec/services/geometry_spec.rb index c9cb9280d..180a0b083 100644 --- a/spec/services/geometry_spec.rb +++ b/spec/services/geometry_spec.rb @@ -46,12 +46,12 @@ # this is the midpoint between stop_a and stop_b, with a little offset target_point = Geometry::DistanceCalculation.cartesian_cast(Stop::GEOFACTORY.point(-121.9664615, 37.36)) locators = cartesian_line.locators(target_point) - i = Geometry::DistanceCalculation.index_of_closest_match_line_segment(locators, 0, locators.size-1, target_point) + i = Geometry::DistanceCalculation.index_of_line_segment_with_nearest_point(locators, 0, locators.size-1) nearest_point = Geometry::LineString.nearest_point_on_line(locators, i) expect(Geometry::LineString.distance_along_line_to_nearest_point(cartesian_line, nearest_point, i)).to be_within(0.1).of(6508.84) end - it '#index_of_closest_match_line_segment' do + it '#index_of_line_segment_with_nearest_point' do coords = @rsp.geometry[:coordinates].concat [stop_b.geometry[:coordinates],stop_a.geometry[:coordinates]] @rsp.geometry = Geometry::LineString.line_string(coords) cartesian_line = Geometry::DistanceCalculation.cartesian_cast(@rsp[:geometry]) @@ -59,7 +59,7 @@ mid = Stop::GEOFACTORY.point(-121.9664615, 37.36) target_point = Geometry::DistanceCalculation.cartesian_cast(mid) locators = cartesian_line.locators(target_point) - i = Geometry::DistanceCalculation.index_of_closest_match_line_segment(locators, 0, locators.size - 1, target_point) + i = Geometry::DistanceCalculation.index_of_line_segment_with_nearest_point(locators, 0, locators.size - 1) expect(i).to eq 0 end @@ -103,7 +103,7 @@ gtfs.trip_stop_times(trips=[trip]){ |trip, stop_times| trip_stop_times = stop_times } #4359.9 is repeated. NOTE: the given shape_dist_traveled may be different from any best match computed distance expect(Geometry::DistanceCalculation.gtfs_shape_dist_traveled(rsp, trip_stop_times, tl_stops, gtfs.shape_line(trip.shape_id).shape_dist_traveled)).to match_array([0.0,399.4,553.4,761.0,906.2,1145.0,1385.2,1586.9,1774.2,2030.3,2214.0,2519.2,2764.1,2885.8,3057.6,3194.0,3429.4,3610.4,4359.9,4359.9,4910.9,5135.1,5363.9,5702.6,5885.2,6103.7,6465.9,6802.6,7415.0,7663.8,8118.1,8358.9,8588.2,8793.5,8963.8,9152.1]) - expect(Geometry::DistanceCalculation.calculate_distances(rsp)).to match_array([0.3,406.7,552.9,761.4,906.0,1144.8,1385.5,1586.9,1774.2,2030.7,2214.5,2519.2,2763.9,2885.8,3057.2,3194.0,3429.1,3610.1,4363.4,4363.8,4915.4,5134.9,5363.7,5702.6,5885.2,6103.7,6465.9,6835.4,7357.5,7672.5,8118.1,8359.0,8588.2,8793.5,8963.4,9151.9]) + expect(Geometry::DistanceCalculation.calculate_distances(rsp)).to match_array([0.3,406.7,552.9,761.0,906.0,1144.8,1385.0,1586.9,1774.2,2030.7,2214.5,2519.3,2763.9,2885.8,3057.2,3194.0,3429.1,3610.1,4363.4,4363.8,4915.4,5134.9,5363.7,5702.6,5885.2,6103.7,6465.9,6769.8,7467.3,7672.5,8118.1,8358.8,8588.2,8793.5,8963.4,9151.9]) end end @@ -186,7 +186,7 @@ it 'accurately calculates the distances of a route with stops along the line that traversed over itself in the opposite direction, but closest match was segment in opposite direction' do @feed, @feed_version = load_feed(feed_version_name: :feed_version_vta_1965654, import_level: 1) - expect(Geometry::DistanceCalculation.calculate_distances(@feed.imported_route_stop_patterns[0])).to match_array([0.0,1490.8,1818.6,2478.0,2928.5,3167.2,3583.3,4079.4,4360.6,4784.1,4970.5,5168.1,5340.5,5599.0,6023.2,6483.9,6770.0,7469.3]) + expect(Geometry::DistanceCalculation.calculate_distances(@feed.imported_route_stop_patterns[0])).to match_array([0.0,1490.8,1818.6,2478.0,2928.5,3167.2,3584.7,4079.4,4360.6,4784.1,4970.5,5168.1,5340.5,5599.0,6023.2,6483.9,6770.0,7469.3]) end it 'calculates the first stop distance correctly' do @@ -362,9 +362,14 @@ expect(Issue.where(issue_type: 'distance_calculation_inaccurate').count).to eq 0 end + it 'calculates distances for case when second stop is close to first segment, but there is a loop between first and second stop' do + feed, feed_version = load_feed(feed_version_name: :feed_version_mbta_33884627, import_level: 1) + expect(RouteStopPattern.first.stop_distances[1]).to eq 327.5 + end + it 'can calculate distances for a closed loop shape where first and last stops are near each other' do feed, feed_version = load_feed(feed_version_name: :feed_version_grand_river_1426033, import_level: 1) - expect(RouteStopPattern.first.stop_distances).to match_array([0.8,616.9,939.8,1381.0,1720.0,2001.1,2245.9,2512.0,2893.3,3387.2,3696.1,4020.1,4158.2,4534.9,5060.1,5354.3,5975.7,6496.3,7200.5,7361.4,7676.4,8228.6,8821.3,9169.3,9922.9,10116.2,10278.6,10650.2,11044.9,11170.4,11642.8,12021.2,12465.0,12796.4,13324.0,13557.6,13852.8,14470.1,14719.8,15156.2,15615.4,15754.6,16002.9,16451.2,16992.8,17266.0,17507.5,17783.6,18193.5,18394.9,18812.1,19063.5,19319.6,19500.8,19920.9,20517.5]) + expect(RouteStopPattern.first.stop_distances).to match_array([0.8,617.8,939.8,1381.0,1720.3,2000.4,2248.0,2515.1,2894.5,3387.2,3696.6,4018.7,4156.4,4534.9,5060.1,5357.4,5977.3,6496.3,7200.5,7362.6,7678.2,8230.4,8818.6,9169.3,9921.2,10113.3,10278.6,10650.2,11044.9,11172.1,11644.4,12022.2,12465.0,12798.3,13324.0,13557.6,13854.5,14470.1,14717.5,15156.8,15615.4,15754.6,16004.2,16451.2,16992.8,17267.9,17507.5,17783.6,18193.5,18394.9,18809.7,19061.2,19319.6,19500.8,19920.9,20517.5]) expect(Issue.where(issue_type: 'distance_calculation_inaccurate').count).to eq 0 end @@ -383,6 +388,11 @@ feed, feed_version = load_feed(feed_version_name: :feed_version_pvta_trip, import_level: 1) expect(Issue.where(issue_type: 'distance_calculation_inaccurate').count).to eq 0 end + + it 'handles short, straight-line, reverse loop' do + feed, feed_version = load_feed(feed_version_name: :feed_version_marta_trip_5449755, import_level: 1) + expect(Issue.where(issue_type: 'distance_calculation_inaccurate').count).to eq 0 + end end context 'determining outlier stops' do diff --git a/spec/support/example_gtfs_archives/marta_trip_5449755.zip b/spec/support/example_gtfs_archives/marta_trip_5449755.zip new file mode 100644 index 000000000..2f4dc89a6 Binary files /dev/null and b/spec/support/example_gtfs_archives/marta_trip_5449755.zip differ diff --git a/spec/support/example_gtfs_archives/mbta_trip_33884115.zip b/spec/support/example_gtfs_archives/mbta_trip_33884115.zip new file mode 100644 index 000000000..4d518beaa Binary files /dev/null and b/spec/support/example_gtfs_archives/mbta_trip_33884115.zip differ diff --git a/spec/support/example_gtfs_archives/mbta_trip_33884627.zip b/spec/support/example_gtfs_archives/mbta_trip_33884627.zip new file mode 100644 index 000000000..0a44814ad Binary files /dev/null and b/spec/support/example_gtfs_archives/mbta_trip_33884627.zip differ