Use osm tags in links reclassification algorithm #3042

genadz · 2021-04-28T21:01:15Z

Issue

This PR fixes the problem described in #2976 .

The main idea is to use ref and destination:ref tags in order to determine actual destination road for the link. It reduces false positive downgrades in cases when we have low-class roads attached in the middle of the link path.

Tasklist

Add tests
Add #fixes with the issue number that this PR addresses
Update the docs with any new request parameters or changes to behavior described
Update the changelog

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

genadz · 2021-04-29T09:34:42Z

test/isochrone.cc

@@ -137,7 +137,7 @@ TEST(Isochrones, Basic) {
    const auto request =
        R"({"locations":[{"lat":52.078937,"lon":5.115321}],"costing":"auto","contours":[{"time":9}],"polygons":false,"generalize":55})";
    const auto expected =
-        R"({"features":[{"properties":{"fill-opacity":0.33,"fill":"#bf4040","fillColor":"#bf4040","color":"#bf4040","fillOpacity":0.33,"opacity":0.33,"contour":9,"metric":"time"},"geometry":{"coordinates":[[5.040321,52.125973],[5.040249,52.125009],[5.039254,52.125004],[5.035884,52.121937],[5.034758,52.119500],[5.033321,52.118968],[5.026649,52.121265],[5.025321,52.123031],[5.023361,52.122977],[5.023263,52.123937],[5.023308,52.122924],[5.025194,52.122810],[5.025247,52.121863],[5.026321,52.120697],[5.031766,52.118382],[5.031800,52.117458],[5.029976,52.116937],[5.030221,52.114037],[5.029321,52.113584],[5.030321,52.113863],[5.030571,52.114687],[5.031410,52.114848],[5.031997,52.116261],[5.034321,52.116540],[5.044321,52.113086],[5.046656,52.111272],[5.049421,52.111037],[5.049845,52.106937],[5.053206,52.103822],[5.057943,52.103559],[5.058321,52.102772],[5.059618,52.102937],[5.059055,52.104203],[5.060634,52.103937],[5.060840,52.098937],[5.060321,52.098312],[5.059554,52.099170],[5.057224,52.098937],[5.059007,52.098623],[5.058997,52.097261],[5.057751,52.096937],[5.060272,52.094888],[5.062321,52.096483],[5.063768,52.093937],[5.063321,52.092391],[5.061321,52.093255],[5.058701,52.092937],[5.058017,52.090241],[5.054983,52.088275],[5.051321,52.088044],[5.048321,52.086875],[5.047321,52.087983],[5.046798,52.086460],[5.045321,52.085858],[5.048774,52.085390],[5.049321,52.083138],[5.049794,52.085464],[5.051766,52.085382],[5.052003,52.084255],[5.050909,52.083937],[5.054201,52.081937],[5.055321,52.082761],[5.071321,52.081586],[5.071556,52.076937],[5.069321,52.075623],[5.068321,52.076586],[5.061002,52.077256],[5.059287,52.073903],[5.060416,52.073842],[5.062321,52.076429],[5.063321,52.075391],[5.066321,52.075319],[5.067613,52.070937],[5.065321,52.069622],[5.060321,52.069593],[5.059321,52.070585],[5.054919,52.070937],[5.056810,52.072937],[5.057875,52.077383],[5.060329,52.078937],[5.059321,52.081147],[5.057797,52.078461],[5.054993,52.077937],[5.055996,52.077612],[5.055660,52.074598],[5.052834,52.074450],[5.053422,52.075937],[5.053321,52.077731],[5.053095,52.076163],[5.051420,52.075937],[5.052009,52.074937],[5.050321,52.072423],[5.049690,52.073937],[5.051236,52.075937],[5.050049,52.077665],[5.047707,52.078323],[5.047589,52.079669],[5.048656,52.079937],[5.048321,52.081425],[5.048144,52.080114],[5.047237,52.080021],[5.046865,52.078393],[5.043259,52.075937],[5.044454,52.074937],[5.043513,52.074745],[5.042771,52.072487],[5.041321,52.072381],[5.040321,52.073407],[5.035321,52.073258],[5.034141,52.077937],[5.029369,52.075937],[5.030321,52.074810],[5.028160,52.075776],[5.022061,52.075197],[5.021982,52.073598],[5.025321,52.073406],[5.026952,52.071568],[5.027082,52.069176],[5.025117,52.068937],[5.025209,52.067937],[5.027321,52.068758],[5.029839,52.071419],[5.033321,52.071573],[5.039288,52.070904],[5.042321,52.069633],[5.046321,52.069539],[5.049321,52.068236],[5.054321,52.068238],[5.056027,52.066937],[5.056321,52.065249],[5.059321,52.067220],[5.063321,52.066206],[5.064688,52.063304],[5.066321,52.061945],[5.067613,52.061937],[5.066349,52.061909],[5.066631,52.058937],[5.065639,52.057937],[5.067760,52.057376],[5.068003,52.054937],[5.068357,52.054901],[5.068843,52.056415],[5.069547,52.056711],[5.069968,52.058290],[5.073608,52.058224],[5.073321,52.057459],[5.071012,52.057246],[5.069750,52.051508],[5.066428,52.052044],[5.066186,52.051072],[5.072978,52.046594],[5.074321,52.047407],[5.075321,52.046378],[5.079321,52.046332],[5.081321,52.047249],[5.082321,52.046270],[5.084321,52.046225],[5.085321,52.047217],[5.089321,52.046308],[5.090321,52.047264],[5.091321,52.046313],[5.093321,52.046314],[5.092964,52.047937],[5.094321,52.048197],[5.098321,52.046251],[5.099146,52.049112],[5.101182,52.051076],[5.102321,52.050149],[5.104099,52.050159],[5.106321,52.050202],[5.107570,52.051186],[5.107870,52.048486],[5.111321,52.048213],[5.112321,52.046219],[5.113321,52.049186],[5.114608,52.048937],[5.113699,52.047937],[5.114893,52.046509],[5.118321,52.046164],[5.120321,52.049163],[5.123321,52.049175],[5.124321,52.048191],[5.125321,52.049185],[5.138321,52.049259],[5.139321,52.050268],[5.141321,52.050285],[5.142321,52.048257],[5.145071,52.052187],[5.146321,52.051295],[5.147321,52.052282],[5.153321,52.052242],[5.154321,52.051234],[5.155792,52.051466],[5.156139,52.054119],[5.159029,52.054937],[5.158321,52.055645],[5.155145,52.055761],[5.155079,52.057179],[5.157321,52.057316],[5.159927,52.058937],[5.157321,52.059556],[5.156321,52.058170],[5.152162,52.058778],[5.151097,52.060713],[5.151688,52.063937],[5.149182,52.063798],[5.149126,52.065937],[5.151199,52.068059],[5.152321,52.068117],[5.157321,52.064146],[5.159112,52.064937],[5.158932,52.066548],[5.155197,52.067813],[5.153958,52.069574],[5.152202,52.069818],[5.151959,52.070937],[5.161802,52.079456],[5.163070,52.087937],[5.161672,52.090937],[5.163074,52.091937],[5.159648,52.093264],[5.157771,52.098387],[5.153858,52.101474],[5.153648,52.102937],[5.161819,52.104937],[5.157321,52.105479],[5.156321,52.104496],[5.154033,52.104649],[5.148684,52.113299],[5.149527,52.117937],[5.148321,52.120051],[5.146534,52.120150],[5.146321,52.122251],[5.146123,52.120135],[5.144825,52.119937],[5.146186,52.119803],[5.146077,52.116937],[5.147974,52.113284],[5.147321,52.112489],[5.146321,52.113215],[5.145118,52.112937],[5.142321,52.110977],[5.143959,52.110575],[5.143963,52.108937],[5.142085,52.108173],[5.141321,52.106324],[5.140321,52.108764],[5.134321,52.106882],[5.133321,52.105352],[5.132854,52.106404],[5.134211,52.106937],[5.131321,52.107348],[5.129321,52.104532],[5.122321,52.105644],[5.121321,52.105183],[5.120646,52.106262],[5.115321,52.106504],[5.111629,52.105937],[5.111321,52.104524],[5.110046,52.107937],[5.109321,52.108407],[5.107983,52.107599],[5.107321,52.109954],[5.107091,52.109167],[5.105126,52.109132],[5.103321,52.106677],[5.101321,52.106736],[5.100321,52.107727],[5.097321,52.105768],[5.096747,52.106937],[5.098469,52.109085],[5.094321,52.107836],[5.092321,52.110126],[5.080321,52.112600],[5.079300,52.111937],[5.081321,52.107534],[5.084321,52.106528],[5.087321,52.107447],[5.089651,52.105607],[5.085321,52.103591],[5.084748,52.104364],[5.082847,52.104463],[5.082407,52.106023],[5.079528,52.105730],[5.078321,52.106797],[5.077321,52.106618],[5.077321,52.104433],[5.078321,52.104674],[5.078969,52.103937],[5.077558,52.102937],[5.080371,52.099937],[5.079321,52.099417],[5.078321,52.100645],[5.077321,52.098424],[5.076321,52.099157],[5.071321,52.098791],[5.070321,52.097363],[5.070073,52.098689],[5.062808,52.105424],[5.058244,52.107860],[5.055535,52.110937],[5.059321,52.112586],[5.062321,52.109844],[5.063234,52.110024],[5.062950,52.111937],[5.057778,52.115937],[5.058321,52.117587],[5.062826,52.117937],[5.062321,52.119025],[5.060321,52.120095],[5.057536,52.119937],[5.059016,52.119632],[5.058321,52.118287],[5.056980,52.119596],[5.055928,52.119544],[5.054321,52.118176],[5.047308,52.116950],[5.049367,52.114983],[5.055683,52.115299],[5.055625,52.113633],[5.053596,52.111662],[5.051321,52.111620],[5.045321,52.114652],[5.043321,52.114448],[5.040321,52.116399],[5.038321,52.116250],[5.035837,52.118453],[5.037321,52.121383],[5.038692,52.121308],[5.039321,52.119998],[5.041321,52.120520],[5.038528,52.122144],[5.038476,52.123782],[5.040367,52.124891],[5.040321,52.125973]],"type":"LineString"},"type":"Feature"}],"type":"FeatureCollection"})";
+        R"({"features":[{"properties":{"fill":"#bf4040","fillOpacity":0.33,"fill-opacity":0.33,"fillColor":"#bf4040","color":"#bf4040","contour":9,"opacity":0.33,"metric":"time"},"geometry":{"coordinates":[[5.040321,52.125973],[5.040249,52.125009],[5.039254,52.125004],[5.035884,52.121937],[5.034758,52.119500],[5.033321,52.118968],[5.026649,52.121265],[5.025321,52.123031],[5.023361,52.122977],[5.023263,52.123937],[5.023308,52.122924],[5.025194,52.122810],[5.025247,52.121863],[5.026321,52.120697],[5.031766,52.118382],[5.031800,52.117458],[5.029976,52.116937],[5.030221,52.114037],[5.029321,52.113584],[5.030321,52.113863],[5.030571,52.114687],[5.031410,52.114848],[5.031997,52.116261],[5.034321,52.116540],[5.044321,52.113086],[5.046656,52.111272],[5.049421,52.111037],[5.049845,52.106937],[5.053206,52.103822],[5.057942,52.103558],[5.058321,52.102772],[5.059618,52.102937],[5.059055,52.104203],[5.060634,52.103937],[5.060839,52.098937],[5.060321,52.098313],[5.059556,52.099172],[5.057209,52.098937],[5.059005,52.098621],[5.058996,52.097262],[5.057744,52.096937],[5.060271,52.094887],[5.062321,52.096482],[5.063768,52.093937],[5.063321,52.092392],[5.061321,52.093256],[5.058695,52.092937],[5.058017,52.090241],[5.054983,52.088275],[5.051321,52.088044],[5.048321,52.086875],[5.047321,52.087983],[5.046798,52.086460],[5.045321,52.085858],[5.048774,52.085390],[5.049321,52.083138],[5.049794,52.085464],[5.051766,52.085382],[5.052003,52.084255],[5.050909,52.083937],[5.054201,52.081937],[5.055321,52.082761],[5.071321,52.081586],[5.071556,52.076937],[5.069321,52.075623],[5.068321,52.076586],[5.061002,52.077256],[5.059287,52.073903],[5.060416,52.073842],[5.062321,52.076429],[5.063321,52.075391],[5.066321,52.075319],[5.067613,52.070937],[5.065321,52.069622],[5.060321,52.069593],[5.059321,52.070585],[5.054919,52.070937],[5.056810,52.072937],[5.057875,52.077383],[5.060329,52.078937],[5.059321,52.081147],[5.057797,52.078461],[5.054993,52.077937],[5.055996,52.077612],[5.055660,52.074598],[5.052834,52.074450],[5.053422,52.075937],[5.053321,52.077731],[5.053095,52.076163],[5.051420,52.075937],[5.052009,52.074937],[5.050321,52.072423],[5.049690,52.073937],[5.051236,52.075937],[5.050049,52.077665],[5.047707,52.078323],[5.047589,52.079669],[5.048656,52.079937],[5.048321,52.081425],[5.048144,52.080114],[5.047237,52.080021],[5.046865,52.078393],[5.043259,52.075937],[5.044454,52.074937],[5.043513,52.074745],[5.043598,52.072937],[5.042321,52.072015],[5.040321,52.073129],[5.035321,52.073258],[5.034141,52.077937],[5.029369,52.075937],[5.030321,52.074810],[5.028160,52.075776],[5.022061,52.075197],[5.021982,52.073598],[5.025321,52.073406],[5.026952,52.071568],[5.027082,52.069176],[5.025117,52.068937],[5.025209,52.067937],[5.027321,52.068758],[5.029839,52.071419],[5.033321,52.071573],[5.046321,52.069539],[5.049321,52.068236],[5.054321,52.068238],[5.056027,52.066937],[5.056321,52.065249],[5.059321,52.067220],[5.063321,52.066206],[5.064688,52.063304],[5.066321,52.061945],[5.067613,52.061937],[5.066349,52.061909],[5.066631,52.058937],[5.065639,52.057937],[5.067760,52.057376],[5.068003,52.054937],[5.068357,52.054901],[5.068843,52.056415],[5.069547,52.056711],[5.069968,52.058290],[5.073608,52.058224],[5.073321,52.057459],[5.071012,52.057246],[5.069750,52.051508],[5.066428,52.052044],[5.066186,52.051072],[5.072978,52.046594],[5.074321,52.047407],[5.075321,52.046378],[5.079321,52.046332],[5.081321,52.047249],[5.082321,52.046270],[5.084321,52.046225],[5.085321,52.047217],[5.089321,52.046308],[5.090321,52.047264],[5.091321,52.046313],[5.093321,52.046314],[5.092964,52.047937],[5.094321,52.048197],[5.098321,52.046251],[5.099146,52.049112],[5.101182,52.051076],[5.102321,52.050149],[5.104099,52.050159],[5.106321,52.050202],[5.107570,52.051186],[5.107870,52.048486],[5.111321,52.048213],[5.112321,52.046219],[5.113321,52.049186],[5.114608,52.048937],[5.113699,52.047937],[5.114893,52.046509],[5.118321,52.046164],[5.120321,52.049163],[5.123321,52.049175],[5.124321,52.048191],[5.125321,52.049185],[5.138321,52.049259],[5.139321,52.050268],[5.141321,52.050285],[5.142321,52.048257],[5.145071,52.052187],[5.146321,52.051295],[5.147321,52.052282],[5.153321,52.052242],[5.154321,52.051234],[5.155792,52.051466],[5.156139,52.054119],[5.159029,52.054937],[5.158321,52.055645],[5.155145,52.055761],[5.155079,52.057179],[5.157321,52.057316],[5.159927,52.058937],[5.157321,52.059556],[5.156321,52.058170],[5.152162,52.058778],[5.151097,52.060713],[5.151688,52.063937],[5.149182,52.063798],[5.149126,52.065937],[5.151199,52.068059],[5.152321,52.068117],[5.157321,52.064146],[5.159112,52.064937],[5.158932,52.066548],[5.155197,52.067813],[5.153958,52.069574],[5.152202,52.069818],[5.151959,52.070937],[5.161802,52.079456],[5.163070,52.087937],[5.161672,52.090937],[5.163074,52.091937],[5.159648,52.093264],[5.157771,52.098387],[5.153858,52.101474],[5.153648,52.102937],[5.161819,52.104937],[5.157321,52.105479],[5.156321,52.104496],[5.154033,52.104649],[5.148684,52.113299],[5.149527,52.117937],[5.148321,52.120051],[5.146534,52.120150],[5.146321,52.122251],[5.146123,52.120135],[5.144825,52.119937],[5.146186,52.119803],[5.146077,52.116937],[5.147974,52.113284],[5.147321,52.112489],[5.146321,52.113215],[5.145118,52.112937],[5.142321,52.110977],[5.143959,52.110575],[5.143963,52.108937],[5.142085,52.108173],[5.141321,52.106324],[5.140321,52.108764],[5.134321,52.106882],[5.133321,52.105352],[5.132854,52.106404],[5.134211,52.106937],[5.131321,52.107348],[5.129321,52.104532],[5.122321,52.105644],[5.121321,52.105183],[5.120646,52.106262],[5.115321,52.106504],[5.111629,52.105937],[5.111321,52.104524],[5.110046,52.107937],[5.109321,52.108407],[5.107983,52.107599],[5.107321,52.109954],[5.107091,52.109167],[5.105126,52.109132],[5.103321,52.106677],[5.101321,52.106736],[5.100321,52.107727],[5.097321,52.105768],[5.096747,52.106937],[5.098469,52.109085],[5.094321,52.107836],[5.092321,52.110126],[5.080321,52.112600],[5.079300,52.111937],[5.081321,52.107534],[5.084321,52.106528],[5.087321,52.107447],[5.089651,52.105607],[5.085321,52.103591],[5.084748,52.104364],[5.082847,52.104463],[5.082407,52.106023],[5.079528,52.105730],[5.078321,52.106797],[5.077321,52.106618],[5.077321,52.104433],[5.078321,52.104674],[5.078969,52.103937],[5.077558,52.102937],[5.080371,52.099937],[5.079321,52.099417],[5.078321,52.100645],[5.077321,52.098424],[5.076321,52.099157],[5.071321,52.098792],[5.070321,52.097363],[5.070074,52.098690],[5.062808,52.105424],[5.058244,52.107860],[5.055535,52.110937],[5.059321,52.112586],[5.062321,52.109844],[5.063233,52.110024],[5.062950,52.111937],[5.057778,52.115937],[5.058321,52.117587],[5.062826,52.117937],[5.062321,52.119025],[5.060321,52.120095],[5.057536,52.119937],[5.059016,52.119632],[5.058321,52.118287],[5.056980,52.119596],[5.055928,52.119544],[5.054321,52.118176],[5.047308,52.116950],[5.049367,52.114983],[5.055683,52.115299],[5.055625,52.113633],[5.053596,52.111662],[5.051321,52.111620],[5.045321,52.114652],[5.043321,52.114448],[5.040321,52.116399],[5.038321,52.116250],[5.035837,52.118453],[5.037321,52.121383],[5.038692,52.121308],[5.039321,52.119998],[5.041321,52.120520],[5.038528,52.122144],[5.038476,52.123782],[5.040367,52.124891],[5.040321,52.125973]],"type":"LineString"},"type":"Feature"}],"type":"FeatureCollection"})";


fixed isochrone test for auto costing model

genadz · 2021-04-29T09:35:57Z

valhalla/mjolnir/linkclassification.h

@@ -15,6 +15,7 @@ void ReclassifyLinks(const std::string& ways_file,
                     const std::string& nodes_file,
                     const std::string& edges_file,
                     const std::string& way_nodes_file,
+                     const OSMData& osmdata,


it's used to extract ref and destination:ref tags

genadz · 2021-04-29T09:38:05Z

test/gurka/test_ramps_tc.cc

+  check_edge_classification(graph_reader, layout, "H", "E", baldr::RoadClass::kTrunk);
+}
+
+TEST(LinkReclassification, test_acyclic_graph) {


check that we correctly handle cases where graph nodes have several inbound edges and several outbound edges

genadz · 2021-04-29T09:41:16Z

test/gurka/test_ramps_tc.cc

+  check_edge_classification(graph_reader, layout, "K", "C", baldr::RoadClass::kTrunk);
+}
+
+TEST(LinkReclassification, test_hierarchical_reclass) {


check that parent links have higher (or equal) classification than their children

genadz · 2021-04-29T09:44:35Z

test/gurka/test_ramps_tc.cc

+}
+} // namespace
+
+TEST(LinkReclassification, test_use_refs) {


check that ref and destination:ref tags are used correctly to detect destination roads. We should use destination road class instead of intermediate roads classes in order to determine final link classification

genadz · 2021-04-29T12:25:08Z

I've tested these changes on the latest osm data for pennsylvania https://download.geofabrik.de/north-america/us/pennsylvania.html

Reclassification statistics:

	reclassified links	turn channels
master	12157	4421
branch	12034	4406

As we can see it's approximately the same; overall there is 149 differences for links reclassification and 17 for turn channels detection. I verified most of them, looks good to me.

Let me share some examples:

	master	branch
https://www.openstreetmap.org/way/197009560	`residential`	`trunk`
https://www.openstreetmap.org/way/448588257	`residential`	`primary`
https://www.openstreetmap.org/way/26404152	`residential`	`trunk`

Run routes

Performance stayed the same (on 10000 random routes in pennsylvania).
Only 37 routes were changed (expected due to the small difference in links reclassification). For all of them ETA decreased or stayed the same.

There is a clear example how incorrect link classification may lead to bad route:
master

branch

* Rewrite algorithm to correcly process acyclic link graph instead of link tree only. * Use ref;destination:ref tags in order to determine actual destination.

kevinkreiser · 2021-04-29T16:51:27Z

src/mjolnir/linkclassification.cc

@@ -9,6 +9,8 @@
 #include "baldr/graphid.h"


at a high level its looks pretty much like a rewrite of the reclassification (which isnt a bad thing) but i was wondering if you might be able to add some comments to this file to draw our attention to the flow of the algorithm. basically explain how this new version works?

@kevinkreiser added some more detailed comments. feel free to ask if you need some more explanations)

kevinkreiser · 2021-04-29T16:52:26Z

test/gurka/test_ramps_tc.cc

+                      |
+                      |
+                      E
+  )";


this diagram wins the award for best ascii art in gurka test 🏆

haha 😂 Tank you!))

gknisely · 2021-04-29T20:07:22Z

src/mjolnir/linkclassification.cc

+      road_tags.dest_refs = GetTagTokens(osmdata.name_offset_map.name(way.destination_ref_index()));
+
+    // Parse 'ref' tag
+    if (way.ref_index() != 0)


@genadz should we be looking at int_ref too?

according to the https://taginfo.openstreetmap.org/tags/highway=motorway#combinations , ref tags is used in more than 94% cases while int_ref is used only in ~23%. For trunk and primary roads we have similar picture (ref tag is much more popular). So, I think int_ref tag won't affect classification too much.

and , in general , we definitely can use other tags (like destination:street; destination:ref:lanes; destination:ref:forward/backward), but all of these require additional testing round. I'd propose in this PR focus only on destination:ref; ref tags

gknisely · 2021-04-29T20:09:11Z

src/mjolnir/linkclassification.cc

+
+    // Parse 'destination:ref' tag
+    if (way.destination_ref_index() != 0)
+      road_tags.dest_refs = GetTagTokens(osmdata.name_offset_map.name(way.destination_ref_index()));


@genadz should we consider looking at destination:ref:to too?

From osm wiki: "destination:ref:to=*'s reference road will never directly connect to the slip road indicated on the sign; it is provided as a means of navigational assistance to the driver."

so, I think it doesn't make sense, we won't reach a destination road following by links

kevinkreiser · 2021-05-03T19:27:37Z

src/mjolnir/linkclassification.cc

+ * nodes where all their children have been processed; 'in_progress' - contains nodes that have been
+ * visited but some of their children haven't been processed yet (in other words 'in progress' set
+ * contains all nodes on the path from the root node to the current node). So, when we reach a node
+ * from the 'processed' set we should and an edge to the graph (that goes from the current node to the


Suggested change

* from the 'processed' set we should and an edge to the graph (that goes from the current node to the

* from the 'processed' set we should add an edge to the graph (that goes from the current node to the

kevinkreiser · 2021-05-03T19:32:23Z

src/mjolnir/linkclassification.cc

+/*
+ * Reclassify links in the acyclic link graph. We maintain a queue of leaf nodes. On each step take
+ * some leaf node from the queue, build a link chain, determine the final road class for the whole
+ * chain (and set the new class), then virtually "remove" reclassified links from the grpaph and


Suggested change

* chain (and set the new class), then virtually "remove" reclassified links from the grpaph and

* chain (and set the new class), then virtually "remove" reclassified links from the graph and

kevinkreiser

I reviewed linkclassification.cc and it looks ok to me, the only thing i would say is can you run the planet with this to be sure there aren't any edge cases (especially with the tree formation maybe?)? It would also be nice to run some cross country or longer routes just to validate that the worst case didnt get a bunch worse or something.

I think its just important to be sure it works for the planet before merging something crucial like that is all.

kevinkreiser · 2021-05-11T17:09:32Z

@genadz this still looks good to me, are you blocked on anything here, maybe re-running RAD since the last commit?

genadz · 2021-05-12T08:42:59Z

@genadz this still looks good to me, are you blocked on anything here, maybe re-running RAD since the last commit?

hey! you're right. I found one bad route when ran RAD tests for the first time. I fixed it and re-running again

genadz · 2021-05-13T19:12:24Z

So, finally I finished with tests. I used the following test files from test_requests directory:

bear_vs_straight_routes.txt
de_benchmark_routes.txt
ferry_routes.txt
fork_bear_routes.txt
internal_edge_routes.txt
md_interchange_routes.txt
oh_interchange_routes.txt
pa_interchange_routes.txt
pedestrian_routes.txt
ramp_divebomb_routes.txt
ramp_fork_routes.txt
ramp_routes.txt
ramp_vs_turn_routes.txt
roundabout_routes.txt
small_end_ramp_forks.txt
turn_channel_routes.txt

and some other test files.

Most changes are minor (usually it's small difference in ETA). Other cases can be divided into 2 groups:

Different routes (with some notable ETA, like >1min).
Different instructions.

Different routes
Most of routes with different ETA were improved, and other have slight ETA increase (not more than 1min).
Example of improved route (save ~15min):

Different instructions
The same as for the previous section, in most case we got small improvements in guidance (thanks @dgearhart for help).
Example of improved guidance:

(because the new algorithm reclassified these links 1, 2 and 3 as trunk_link while the old one reclassified them as residential)

We also detected one bad example:

The new algorithm didn't downgrade these links 1 and 2 , but master reclassified them as tertiary. It affected guidance in a bad way.

As far as we definitely see positive dynamic we've decided to merge current branch and try to fix the last example in a new PR.

dgearhart

Some nice improvements - thanks @genadz

genadz marked this pull request as draft April 28, 2021 21:01

genadz force-pushed the kgv_links_reclass_use_refs branch 2 times, most recently from ee749c3 to 6e3c2dd Compare April 29, 2021 09:21

genadz commented Apr 29, 2021

View reviewed changes

genadz marked this pull request as ready for review April 29, 2021 09:54

genadz requested review from dgearhart, dnesbitt61 and kevinkreiser April 29, 2021 09:54

genadz added 4 commits April 29, 2021 18:18

Use osm tags in link reclassification algorithm

553c7d8

* Rewrite algorithm to correcly process acyclic link graph instead of link tree only. * Use ref;destination:ref tags in order to determine actual destination.

Add tests for links reclassification

025cd3c

Fix isochrone test

6304e02

Update changelog

8c727e2

genadz force-pushed the kgv_links_reclass_use_refs branch from 6e3c2dd to 8c727e2 Compare April 29, 2021 15:18

Merge branch 'master' into kgv_links_reclass_use_refs

59922fd

kevinkreiser reviewed Apr 29, 2021

View reviewed changes

gknisely reviewed Apr 29, 2021

View reviewed changes

genadz added 2 commits April 30, 2021 12:56

Add more comments about how new reclassification algorithm works

68b9ae0

Merge branch 'master' into kgv_links_reclass_use_refs

688e9bb

genadz requested a review from merkispavel April 30, 2021 13:22

genadz added 3 commits May 3, 2021 14:07

Merge branch 'master' into kgv_links_reclass_use_refs

cb225c2

Add more comments to the tests

730bf8e

Merge branch 'master' into kgv_links_reclass_use_refs

bd0a89d

kevinkreiser reviewed May 3, 2021

View reviewed changes

kevinkreiser previously approved these changes May 4, 2021

View reviewed changes

Fix misprints

f3308ce

genadz dismissed kevinkreiser’s stale review via f3308ce May 4, 2021 08:06

genadz requested a review from gknisely May 4, 2021 08:08

Compare link refs with nonlink refs to detect a destination

3c88fca

Merge branch 'master' into kgv_links_reclass_use_refs

8f17b2e

dgearhart approved these changes May 13, 2021

View reviewed changes

genadz merged commit edf42ea into master May 13, 2021

genadz deleted the kgv_links_reclass_use_refs branch July 1, 2021 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use osm tags in links reclassification algorithm #3042

Use osm tags in links reclassification algorithm #3042

genadz commented Apr 28, 2021 •

edited

Loading

genadz Apr 29, 2021

genadz Apr 29, 2021

genadz Apr 29, 2021

genadz Apr 29, 2021

genadz Apr 29, 2021

genadz commented Apr 29, 2021

kevinkreiser Apr 29, 2021

genadz Apr 30, 2021

genadz Apr 30, 2021

kevinkreiser Apr 29, 2021

genadz Apr 29, 2021

gknisely Apr 29, 2021

genadz Apr 30, 2021

gknisely Apr 29, 2021

genadz Apr 30, 2021

kevinkreiser May 3, 2021

kevinkreiser May 3, 2021

kevinkreiser left a comment

kevinkreiser commented May 11, 2021

genadz commented May 12, 2021

genadz commented May 13, 2021

dgearhart left a comment

	* from the 'processed' set we should and an edge to the graph (that goes from the current node to the
	* from the 'processed' set we should add an edge to the graph (that goes from the current node to the

	* chain (and set the new class), then virtually "remove" reclassified links from the grpaph and
	* chain (and set the new class), then virtually "remove" reclassified links from the graph and

Use osm tags in links reclassification algorithm #3042

Use osm tags in links reclassification algorithm #3042

Conversation

genadz commented Apr 28, 2021 • edited Loading

Issue

Tasklist

Requirements / Relations

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

genadz commented Apr 29, 2021

Run routes

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

kevinkreiser left a comment

Choose a reason for hiding this comment

kevinkreiser commented May 11, 2021

genadz commented May 12, 2021

genadz commented May 13, 2021

dgearhart left a comment

Choose a reason for hiding this comment

genadz commented Apr 28, 2021 •

edited

Loading