Improve Turn Angles #2779

MoKob · 2016-08-17T07:51:03Z

This PR addresses the improvement of turn angles raised in #2778.

resolves in #2778
resolves #2937

TODO

Right now I see multiple strategies that I will update the PR with to report on my findings:
~~- [ ] Average Turn Angle among multiple coordinates~~

MoKob · 2016-08-17T10:55:27Z

I've implemented a first crack at resolving #2778 . The change results in updated turn angles on Berlin in many cases. If only printing changes of at least 20 degrees, we see changes reflected in the following list:

Changed Angle from 75.6269169146 to 97.6369878691 at: 52.537318 13.360354
Changed Angle from 289.518196384 to 309.945525292 at: 52.522206 13.587492
Changed Angle from 129.710002289 to 153.400320439 at: 52.475872 13.535999
Changed Angle from 247.51201648 to 225.000686656 at: 52.452798 13.560956
Changed Angle from 307.748912795 to 335.563286793 at: 52.450324 13.51931
Changed Angle from 308.98626688 to 287.008468757 at: 52.512135 13.390056
Changed Angle from 235.577935454 to 256.583428702 at: 52.482608 13.349332
Changed Angle from 200.673838407 to 180 at: 52.472489 13.384549

I am verifying this list to see whether these changes can be considered reasonable or not.

Edit: I've reduced the list to a set of problematic cases I've seen. Further verification is ongoing.

MoKob · 2016-08-17T14:38:12Z

From what I've seen in the changes: the most angles affected very strongly are turn-lanes in between two ways representing the same road, roads splitting into two ways and turning roads/traffic circles.

The affected angles for splitting roads are most times alright, but can offer too strong variations in my view. I have to check more items to see the effect in a global scenario.

Any curve in a road is usually difficult to begin with. The angles seem a bit off to me. Will need to investigate this further.

MoKob · 2016-08-29T14:04:28Z

local request

should be turn-left (requires angle adjustment in collapsing)

MoKob · 2016-08-29T14:11:46Z

Local requests broken at link road:
here
here, and here

MoKob · 2016-08-30T11:28:58Z

MoKob · 2016-09-21T11:48:08Z

#2937 is somewhat standing in our way.

MoKob · 2016-09-28T15:14:24Z

Apparently one of the tests only succeeded due to a sliproad detection. This detection has changed. The test needs investigation to make it pass again.

MoKob · 2016-10-04T08:15:00Z

First Coordinate Far Away - Use the first, since the turn cannot be simply an offset
Straight Line Heuristic - Turn is straight, any coordinate would be fine for the turn angle
Starts of without turn - Road takes a turn within the first 30 meters, but the start is straight
Regression Line - The road itself does not deviate far from a straight line, but enough to consider it. We choose between:
- First Coordinate
  - And offsetting (moving the straight line to the intersection for the angle) none on Berlin
Offset Coordinates - A short initial segment, bridging lanes usually
The initial offset can happen in a tiny curve as well
Curves usually are described best by their very first index:

MoKob · 2016-10-04T11:27:03Z

Circle Offsets are currently offering strange results. Needs further investigation.

MoKob · 2016-10-04T13:08:40Z

Example Picture of how angle computation can change

daniel-j-h · 2016-10-04T13:14:43Z

@MoKob care to elaborate on these images? Color reflects angle?

MoKob · 2016-10-04T13:16:51Z

Greenish colours are coordinates of the connected roads. Blue are the three coordinates chosen to calculate the turn angle. On the satellite image you can see that the angles better describe the actual turn here.

MoKob · 2016-10-06T15:01:18Z

another example: now 260 degree turn, previous 220 degree turn

MoKob · 2016-10-12T07:38:42Z

features/support/route.js

-        return this.extractInstructionList(instructions, s => s.maneuver.bearing_before + '->' + s.maneuver.bearing_after);
+        return this.extractInstructionList(instructions, s => ('in' in s.intersections[0] ? this.reverseBearing(s.intersections[0].bearings[s.intersections[0].in]) : 0)
+                                                              + '->' +
+                                                              ('out' in s.intersections[0] ? s.intersections[0].bearings[s.intersections[0].out] : 0));


this change uses the bearing array instead of the parameters we want to deprecate anyhow.

MoKob · 2016-10-12T07:39:41Z

include/engine/routing_algorithms/routing_base.hpp

+                                 INVALID_ENTRY_CLASSID,
+                                 datasource_vector[segment_idx],
+                                 util::guidance::TurnBearing(0),
+                                 util::guidance::TurnBearing(0)});


the rest of this part was not aligned properly. The addition of the turn-bearings is the only change here.

MoKob · 2016-10-12T07:39:52Z

include/engine/routing_algorithms/routing_base.hpp

+                unpacked_path.back().turn_instruction = turn_instruction;
+                unpacked_path.back().duration_until_turn += (edge_data.distance - total_weight);
+                unpacked_path.back().pre_turn_bearing = facade.PreTurnBearing(edge_data.id);
+                unpacked_path.back().post_turn_bearing = facade.PostTurnBearing(edge_data.id);


... and here

MoKob · 2016-10-12T07:40:38Z

include/extractor/guidance/constants.hpp

@@ -34,7 +34,7 @@ const int constexpr MAX_SLIPROAD_THRESHOLD = 250;
 // Road priorities give an idea of how obvious a turn is. If two priorities differ greatly (e.g.
 // service road over a primary road, the better priority can be seen as obvious due to its road
 // category).
-const double constexpr PRIORITY_DISTINCTION_FACTOR = 2.0;
+const double constexpr PRIORITY_DISTINCTION_FACTOR = 1.75;


turned out the obvious detection was a bit to restrictive here

daniel-j-h

Trophy for best ASCII Cucumber scenarios! More seriously, wow those are detailed and describe their OSM link - thanks for that!

Most of the (i, i+-1) code could benefit from some stdlib algorithms (check algorithms and numeric header).

I think the two major issues are possible iterator invalidation and the accumulate with int.

daniel-j-h · 2016-10-12T15:20:06Z

features/guidance/anticipate-lanes.feature

+               | de    |                                                 | bot  | primary | yes    |
+               | dr    |                                                 | off  | primary | yes    |
+               | gj    |                                                 | bot  | primary | yes    |
+               | jk    |                                                 | bot  | primary | yes    |


Thanks for fixing!

daniel-j-h · 2016-10-12T15:55:58Z

features/guidance/collapse.feature

-            | waypoints | route                   | turns                                                                                  |
-            | k,j       | on,ferry,,ferry,off,off | depart,notification straight,continue uturn,turn straight,notification straight,arrive |
+            | waypoints | route                            | turns                                                                                                        |
+            | k,j       | on,ferry,road,road,ferry,off,off | depart,notification straight,notification straight,continue uturn,turn straight,notification straight,arrive |


The changes in route and turns here is not only related to only ga being a ferry now?

the change is including ab as non-ferry. The problem we were trying to check here was having mode-changes (notifications) visible. This was not the case without the change anymore.

daniel-j-h · 2016-10-12T16:05:39Z

features/guidance/turn-angles.feature

+                              |
+                              |
+                              |
+                              d


The ~7 meter here is compensated by abc having 4 lanes, right?

daniel-j-h · 2016-10-12T16:06:29Z

features/guidance/turn-angles.feature

+                              |
+                              |
+                              |
+                              d


.. and no longer here since it's still 4 lanes but more like 20 meters?

Exactly. Lanes influence how a road is seen. If the modelled road takes a turn right after it reaches the border of the road, the turn itself is most likely describing a typical phenomenon in osm which i modelled in #2778 (comment).

daniel-j-h · 2016-10-12T16:12:59Z

features/guidance/turn-angles.feature

+       When I route I should get
+            | waypoints | route          | turns                        |
+            | a,b       | road,road      | depart,arrive                |
+            | a,i       | road,road,road | depart,continue uturn,arrive |


Hm is there a limit on the uturn's road length? I can see how this is a uturn with this small of a grid size, what about larger ones?

Yes. There is a limit on how long this road can become.

daniel-j-h · 2016-10-13T12:55:15Z

src/extractor/guidance/coordinate_extractor.cpp

+            const auto slope = dividend / divisor;
+            const auto intercept = (sum_lat - slope * sum_lon) / coordinates.size();
+
+            const auto GetLatatLon =


daniel-j-h · 2016-10-13T12:58:32Z

src/extractor/guidance/turn_handler.cpp

        }
    }
    else
    {
-        if (angularDeviation(left.turn.angle, 270) > angularDeviation(right.turn.angle, 270))
+        if (angularDeviation(left.turn.angle, 265) >= angularDeviation(right.turn.angle, 265))


Oh? Can you explain the changes for the lines above?

essentially gives a little bit of wiggle room. 270 is perfectly left.

daniel-j-h · 2016-10-13T13:00:41Z

src/extractor/scripting_environment_lua.cpp

+                       &guidance::RoadClassification::SetClass)
+             .property("num_lanes",
+                       &guidance::RoadClassification::GetNumberOfLanes,
+                       &guidance::RoadClassification::SetNumberOfLanes),


There are road width tags (e.g. http://taginfo.openstreetmap.org/keys/width#combinations) - do you think it could make sense to use those for shortcutting the width calculation via lanes?

Added to #3055

daniel-j-h · 2016-10-13T13:01:32Z

src/util/coordinate_calculation.cpp

+                  static_cast<double>(toFloating(first_coordinate.lon)) *
+                      static_cast<double>(toFloating(third_coordinate.lat)) +
+                  static_cast<double>(toFloating(second_coordinate.lon)) *
+                      static_cast<double>(toFloating(third_coordinate.lat)));


Hm. Maybe make some tmp. vars for the calculation, this is somewhat hard to read.

daniel-j-h · 2016-10-13T13:02:48Z

taginfo.json

@@ -188,6 +188,8 @@
        {"key": "highway", "value": "default"},
        {"key": "width", "description": "Penalties for narrow streets"},
        {"key": "lanes", "description": "Penalties for shared single lane streets"},
+        {"key": "lanes:forward", "description": "Lanes in forward direction"},
+        {"key": "lanes:back", "description": "Lanes in backward direction"},


Hm I can't see those getting used, also it should be backward I suppose?

MoKob · 2016-10-19T08:07:48Z

@daniel-j-h what about the changes? Would love to get this off my plate

daniel-j-h · 2016-10-19T18:15:39Z

Thank you for the adjustments.

This PR adds more advanced coordinate extraction, analysing the road to detect offsets due to OSM way modelling. In addition it improves the handling of bearings. Right now OSM reports bearings simply based on the very first coordinate along a way. With this PR, we store the bearings for a turn correctly, making the bearings for turns correct.

MoKob added the Guidance label Aug 17, 2016

MoKob self-assigned this Aug 17, 2016

MoKob mentioned this pull request Aug 17, 2016

Improve Turn Angles #2778

Closed

daniel-j-h mentioned this pull request Aug 22, 2016

Scenario for unnecessary slight right onto Holzmarktstr due to node offset #2752

Merged

6 tasks

MoKob force-pushed the guidance/2778 branch from 1349811 to f3c85e0 Compare August 24, 2016 15:12

MoKob added this to the 5.4.0 milestone Sep 2, 2016

MoKob mentioned this pull request Sep 2, 2016

Allow Turning from None-Lanes / Consider Target Road Lanes #2645

Open

MoKob changed the title ~~[wip] fix #2778~~ fix #2778 Sep 2, 2016

MoKob added the Work In Progress label Sep 2, 2016

MoKob modified the milestones: 5.5.0, 5.4.0 Sep 6, 2016

MoKob force-pushed the guidance/2778 branch 2 times, most recently from 3b89d51 to 7c15d0b Compare September 7, 2016 13:07

MoKob force-pushed the guidance/2778 branch from 54819fb to 2eab4c6 Compare September 13, 2016 11:29

MoKob changed the title ~~fix #2778~~ Improve Turn Angles Sep 27, 2016

MoKob force-pushed the guidance/2778 branch from 1139615 to 2497409 Compare September 28, 2016 15:13

MoKob force-pushed the guidance/2778 branch from 2497409 to 87482f8 Compare September 28, 2016 15:38

MoKob force-pushed the guidance/2778 branch from f9ffe08 to 76c0ee8 Compare October 4, 2016 08:25

MoKob force-pushed the guidance/2778 branch from bd8da44 to 78c7788 Compare October 6, 2016 12:50

MoKob force-pushed the guidance/2778 branch from 0f7a9f4 to d372f1b Compare October 11, 2016 10:52

MoKob commented Oct 12, 2016

View reviewed changes

MoKob force-pushed the guidance/2778 branch from 36c0320 to 5333017 Compare October 12, 2016 07:46

MoKob added Review and removed Work In Progress labels Oct 12, 2016

daniel-j-h requested changes Oct 13, 2016

View reviewed changes

MoKob force-pushed the guidance/2778 branch 2 times, most recently from d3bfae5 to bee0614 Compare October 17, 2016 06:59

MoKob added Ready To Merge and removed Review labels Oct 18, 2016

This was referenced Oct 19, 2016

Revisit Merging of Segregated Roads/Joining Road Adjustments #3107

Closed

Refactor Roundabout Handler #3108

Closed

daniel-j-h approved these changes Oct 19, 2016

View reviewed changes

MoKob force-pushed the guidance/2778 branch from bee0614 to e3eddb7 Compare October 20, 2016 08:36

MoKob force-pushed the guidance/2778 branch from e3eddb7 to 5e167b8 Compare October 20, 2016 08:47

MoKob merged commit 5e167b8 into master Oct 20, 2016

MoKob deleted the guidance/2778 branch October 20, 2016 09:06

Improve Turn Angles #2779

Improve Turn Angles #2779

Conversation

MoKob commented Aug 17, 2016 • edited Loading

TODO

MoKob commented Aug 17, 2016 • edited Loading

MoKob commented Aug 17, 2016

MoKob commented Aug 29, 2016

MoKob commented Aug 29, 2016 • edited Loading

MoKob commented Aug 30, 2016 • edited Loading

MoKob commented Sep 21, 2016 • edited Loading

MoKob commented Sep 28, 2016

MoKob commented Oct 4, 2016

MoKob commented Oct 4, 2016

MoKob commented Oct 4, 2016

daniel-j-h commented Oct 4, 2016

MoKob commented Oct 4, 2016

MoKob commented Oct 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-j-h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MoKob commented Oct 19, 2016

daniel-j-h commented Oct 19, 2016

MoKob commented Aug 17, 2016 •

edited

Loading

MoKob commented Aug 17, 2016 •

edited

Loading

MoKob commented Aug 29, 2016 •

edited

Loading

MoKob commented Aug 30, 2016 •

edited

Loading

MoKob commented Sep 21, 2016 •

edited

Loading