From 416369c3b823c990ec448a07d4aaff94be199185 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 7 Oct 2021 10:28:03 -0600 Subject: [PATCH 01/23] trying a diff angle for fork --- src/odin/enhancedtrippath.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odin/enhancedtrippath.cc b/src/odin/enhancedtrippath.cc index 2eb3dbd4aa..d16266ecaa 100644 --- a/src/odin/enhancedtrippath.cc +++ b/src/odin/enhancedtrippath.cc @@ -176,7 +176,7 @@ const std::string& TripLeg_Sidewalk_Name(int v) { // TODO: in the future might have to have dynamic angle based on road class and lane count bool is_fork_forward(uint32_t turn_degree) { - return ((turn_degree > 339) || (turn_degree < 21)); + return ((turn_degree > 345) || (turn_degree < 15)); } bool is_relative_straight(uint32_t turn_degree) { From f5c4a7069e999ccfb61a1e68e97c03e2e5647daa Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 14 Oct 2021 16:35:51 -0600 Subject: [PATCH 02/23] Trying something --- src/odin/maneuversbuilder.cc | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index e7bdef4d67..bb481277b9 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2308,8 +2308,16 @@ bool ManeuversBuilder::IsFork(int node_index, // 5 lanes split into 3 lanes left and 3 lanes right // 6 lanes split into 3 lanes left and 3 lanes right // ... - auto has_lane_bifurcation = [](uint32_t prev_lane_count, uint32_t curr_lane_count, + auto has_lane_bifurcation = [](const EnhancedTripLeg_Edge* prev_edge, + const EnhancedTripLeg_Edge* curr_edge, uint32_t xedge_lane_count) -> bool { + uint32_t prev_lane_count = prev_edge->lane_count(); + uint32_t curr_lane_count = curr_edge->lane_count(); + // Going from N+1 lanes to N lanes by virtue of a deceleration/exit lane. + if (prev_lane_count == curr_lane_count + 1) { + if (prev_edge->HasTurnLane(kTurnLaneSlightRight) || prev_edge->HasTurnLane(kTurnLaneSlightLeft)) + return false; + } uint32_t post_split_min_count = (prev_lane_count + 1) / 2; if ((prev_lane_count == 2) && (curr_lane_count == 1) && (xedge_lane_count == 1)) { return true; @@ -2325,10 +2333,9 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || (xedge->IsHighway() && curr_edge->IsRampUse())) && - prev_edge->IsForkForward( - GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && - prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading())) && - has_lane_bifurcation(prev_edge->lane_count(), curr_edge->lane_count(), xedge->lane_count())) { + prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && + has_lane_bifurcation(prev_edge, curr_edge, xedge->lane_count()) && + prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { return true; } } From 3a662d193d94972a06d321e5370dc0919cd26464 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 14 Oct 2021 16:37:53 -0600 Subject: [PATCH 03/23] f --- src/odin/maneuversbuilder.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 18788c041d..a92390abb4 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2349,7 +2349,8 @@ bool ManeuversBuilder::IsFork(int node_index, uint32_t curr_lane_count = curr_edge->lane_count(); // Going from N+1 lanes to N lanes by virtue of a deceleration/exit lane. if (prev_lane_count == curr_lane_count + 1) { - if (prev_edge->HasTurnLane(kTurnLaneSlightRight) || prev_edge->HasTurnLane(kTurnLaneSlightLeft)) + if (prev_edge->HasTurnLane(kTurnLaneSlightRight) || + prev_edge->HasTurnLane(kTurnLaneSlightLeft)) return false; } uint32_t post_split_min_count = (prev_lane_count + 1) / 2; @@ -2367,7 +2368,8 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || (xedge->IsHighway() && curr_edge->IsRampUse())) && - prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && + prev_edge->IsForkForward( + GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && has_lane_bifurcation(prev_edge, curr_edge, xedge->lane_count()) && prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { return true; From 12be03202ae07ff491021cd5ce69ed39aa1446bd Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 19 Oct 2021 12:45:22 -0600 Subject: [PATCH 04/23] stuff --- src/odin/enhancedtrippath.cc | 2 +- src/odin/maneuversbuilder.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/odin/enhancedtrippath.cc b/src/odin/enhancedtrippath.cc index acf2ad9e76..be4c596986 100644 --- a/src/odin/enhancedtrippath.cc +++ b/src/odin/enhancedtrippath.cc @@ -189,7 +189,7 @@ const std::string& TripLeg_Sidewalk_Name(int v) { // TODO: in the future might have to have dynamic angle based on road class and lane count bool is_fork_forward(uint32_t turn_degree) { - return ((turn_degree > 345) || (turn_degree < 15)); + return ((turn_degree > 339) || (turn_degree < 21)); } bool is_relative_straight(uint32_t turn_degree) { diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index a92390abb4..a1ab561c19 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2348,7 +2348,7 @@ bool ManeuversBuilder::IsFork(int node_index, uint32_t prev_lane_count = prev_edge->lane_count(); uint32_t curr_lane_count = curr_edge->lane_count(); // Going from N+1 lanes to N lanes by virtue of a deceleration/exit lane. - if (prev_lane_count == curr_lane_count + 1) { + if ((prev_lane_count == curr_lane_count + 1) && (curr_lane_count > 1)) { if (prev_edge->HasTurnLane(kTurnLaneSlightRight) || prev_edge->HasTurnLane(kTurnLaneSlightLeft)) return false; From 4348494f08e406f274b619a2c4c5b988a1eb7777 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Fri, 22 Oct 2021 16:35:12 -0600 Subject: [PATCH 05/23] trying something --- src/odin/maneuversbuilder.cc | 53 +++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index a1ab561c19..39450e74e2 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2236,6 +2236,21 @@ bool ManeuversBuilder::IsMergeManeuverType(Maneuver& maneuver, return false; } +// return the length in km for a deceleration lane as a function of the road's speed +float get_deceleration_lane_length(float speed_kph) { + if (speed_kph > 112) { + return 0.213; + } + if (speed_kph > 96) { + return 0.183; + } + if (speed_kph > 80) { + return 0.152; + } + + return 0.122; +} + bool ManeuversBuilder::IsFork(int node_index, EnhancedTripLeg_Edge* prev_edge, EnhancedTripLeg_Edge* curr_edge) const { @@ -2342,17 +2357,37 @@ bool ManeuversBuilder::IsFork(int node_index, // 5 lanes split into 3 lanes left and 3 lanes right // 6 lanes split into 3 lanes left and 3 lanes right // ... - auto has_lane_bifurcation = [](const EnhancedTripLeg_Edge* prev_edge, - const EnhancedTripLeg_Edge* curr_edge, - uint32_t xedge_lane_count) -> bool { + auto has_lane_bifurcation = + [](EnhancedTripLeg* trip_path, int node_index, const EnhancedTripLeg_Edge* prev_edge, + const EnhancedTripLeg_Edge* curr_edge, + const std::unique_ptr& xedge) -> bool { uint32_t prev_lane_count = prev_edge->lane_count(); uint32_t curr_lane_count = curr_edge->lane_count(); - // Going from N+1 lanes to N lanes by virtue of a deceleration/exit lane. - if ((prev_lane_count == curr_lane_count + 1) && (curr_lane_count > 1)) { - if (prev_edge->HasTurnLane(kTurnLaneSlightRight) || - prev_edge->HasTurnLane(kTurnLaneSlightLeft)) + uint32_t xedge_lane_count = xedge->lane_count(); + + // Going from N+1 lanes to N lanes. See if previous-lanes appear to be a + // deceleration lane. Iterate prev's until we've exceeded the length + // of a deceleration lane (as a function of speed). If the highway at + // that point has gone down a lane, then it is likely that prev-edge is + // a deceleration lane - which means this is more likely an exit than + // a highway bifurcation. + if ((curr_lane_count > 1) && (prev_lane_count == curr_lane_count + 1) && + (xedge->use() == TripLeg_Use_kRampUse)) { + int delta = 1; + auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); + float deceleration_lane_length = get_deceleration_lane_length(prev_at_delta->default_speed()); + float agg_lane_length_ = prev_at_delta->length_km(); + while (agg_lane_length_ < deceleration_lane_length) { + delta++; + prev_at_delta = trip_path->GetPrevEdge(node_index, delta); + agg_lane_length_ += prev_at_delta->length_km(); + } + + if (prev_at_delta->lane_count() == curr_lane_count) { return false; + } } + uint32_t post_split_min_count = (prev_lane_count + 1) / 2; if ((prev_lane_count == 2) && (curr_lane_count == 1) && (xedge_lane_count == 1)) { return true; @@ -2363,14 +2398,12 @@ bool ManeuversBuilder::IsFork(int node_index, return false; }; - // The curr edge and intersecting edge must be highway or ramp but not both - // validate lane bifurcation if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || (xedge->IsHighway() && curr_edge->IsRampUse())) && + has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && prev_edge->IsForkForward( GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && - has_lane_bifurcation(prev_edge, curr_edge, xedge->lane_count()) && prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { return true; } From bc326a484a317a8f7ebcec8b0747f5494f9ef157 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Mon, 25 Oct 2021 16:43:33 -0600 Subject: [PATCH 06/23] safer --- src/odin/maneuversbuilder.cc | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 39450e74e2..684c1455bf 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2363,32 +2363,37 @@ bool ManeuversBuilder::IsFork(int node_index, const std::unique_ptr& xedge) -> bool { uint32_t prev_lane_count = prev_edge->lane_count(); uint32_t curr_lane_count = curr_edge->lane_count(); - uint32_t xedge_lane_count = xedge->lane_count(); - // Going from N+1 lanes to N lanes. See if previous-lanes appear to be a - // deceleration lane. Iterate prev's until we've exceeded the length - // of a deceleration lane (as a function of speed). If the highway at - // that point has gone down a lane, then it is likely that prev-edge is - // a deceleration lane - which means this is more likely an exit than - // a highway bifurcation. + // Going from N+1 lanes to N lanes. Is this really a highway bifurcation or + // just a deceleration lane? if ((curr_lane_count > 1) && (prev_lane_count == curr_lane_count + 1) && (xedge->use() == TripLeg_Use_kRampUse)) { + + // See if previous-lanes appear to be a deceleration lane. A deceleration + // lane will have a limited length. Iterate previous edges until we've + // exceeded the length of a deceleration lane. If the highway at + // that point has gone down a lane, then it is likely that prev-edge is + // a deceleration lane - which means this is more likely an exit than + // a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); float deceleration_lane_length = get_deceleration_lane_length(prev_at_delta->default_speed()); - float agg_lane_length_ = prev_at_delta->length_km(); - while (agg_lane_length_ < deceleration_lane_length) { + float agg_lane_length_km = prev_at_delta->length_km(); + while (agg_lane_length_km < deceleration_lane_length) { delta++; prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - agg_lane_length_ += prev_at_delta->length_km(); + if (!prev_at_delta) + break; + agg_lane_length_km += prev_at_delta->length_km(); } - if (prev_at_delta->lane_count() == curr_lane_count) { + if (prev_at_delta && (prev_at_delta->lane_count() == curr_lane_count)) { return false; } } uint32_t post_split_min_count = (prev_lane_count + 1) / 2; + uint32_t xedge_lane_count = xedge->lane_count(); if ((prev_lane_count == 2) && (curr_lane_count == 1) && (xedge_lane_count == 1)) { return true; } else if ((prev_lane_count > 2) && (curr_lane_count == post_split_min_count) && From 312a8cd8bd3aa825cd0599b9608956501b3ca77d Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 26 Oct 2021 14:37:20 -0600 Subject: [PATCH 07/23] comments --- src/odin/maneuversbuilder.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 684c1455bf..3cb4a54db8 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2236,7 +2236,14 @@ bool ManeuversBuilder::IsMergeManeuverType(Maneuver& maneuver, return false; } -// return the length in km for a deceleration lane as a function of the road's speed +// return the length in km for a deceleration lane as a function of the road's speed. +// +// speed (mph) | decel lane length +// -----------------|-------------------- +// >70 | 700 ft +// 60-70 | 600 ft +// 50-60 | 500 ft +// <50 | 400 ft float get_deceleration_lane_length(float speed_kph) { if (speed_kph > 112) { return 0.213; @@ -2365,16 +2372,16 @@ bool ManeuversBuilder::IsFork(int node_index, uint32_t curr_lane_count = curr_edge->lane_count(); // Going from N+1 lanes to N lanes. Is this really a highway bifurcation or - // just a deceleration lane? + // just a deceleration lane for an exit? if ((curr_lane_count > 1) && (prev_lane_count == curr_lane_count + 1) && (xedge->use() == TripLeg_Use_kRampUse)) { - // See if previous-lanes appear to be a deceleration lane. A deceleration - // lane will have a limited length. Iterate previous edges until we've - // exceeded the length of a deceleration lane. If the highway at - // that point has gone down a lane, then it is likely that prev-edge is - // a deceleration lane - which means this is more likely an exit than - // a highway bifurcation. + // Determine if this lane split is a deceleration lane forking onto an + // exit. A deceleration lane will have a predictable length. Iterate previous + // edges until we've exceeded the length of a deceleration lane. If the + // highway at that point has gone down a lane, then it is likely that + // prev-edge is a deceleration lane - which means this is more likely + // an exit than a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); float deceleration_lane_length = get_deceleration_lane_length(prev_at_delta->default_speed()); From 0f1eef9a8148aafabbb4f6cb56eb23681abe5d4d Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Fri, 29 Oct 2021 15:23:27 -0600 Subject: [PATCH 08/23] var rename --- src/odin/maneuversbuilder.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 3cb4a54db8..79985711ac 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2384,9 +2384,9 @@ bool ManeuversBuilder::IsFork(int node_index, // an exit than a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - float deceleration_lane_length = get_deceleration_lane_length(prev_at_delta->default_speed()); + float deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); float agg_lane_length_km = prev_at_delta->length_km(); - while (agg_lane_length_km < deceleration_lane_length) { + while (agg_lane_length_km < deceleration_lane_length_km) { delta++; prev_at_delta = trip_path->GetPrevEdge(node_index, delta); if (!prev_at_delta) From bc7748085774c84575f03ea436990d86f28afa1e Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Mon, 1 Nov 2021 16:12:45 -0600 Subject: [PATCH 09/23] latest, tests are a wip --- src/odin/maneuversbuilder.cc | 28 ++++++-- test/gurka/test_forks.cc | 136 +++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 test/gurka/test_forks.cc diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 79985711ac..bedcf9d4c0 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2258,6 +2258,11 @@ float get_deceleration_lane_length(float speed_kph) { return 0.122; } +// This is a new idea... see usage. +bool is_nearly_straight(uint32_t turn_degree) { + return ((turn_degree > 357) || (turn_degree < 3)); +} + bool ManeuversBuilder::IsFork(int node_index, EnhancedTripLeg_Edge* prev_edge, EnhancedTripLeg_Edge* curr_edge) const { @@ -2384,7 +2389,9 @@ bool ManeuversBuilder::IsFork(int node_index, // an exit than a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - float deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); + float kt = prev_at_delta->speed(); + float deceleration_lane_length_km = + get_deceleration_lane_length(prev_at_delta->default_speed()); float agg_lane_length_km = prev_at_delta->length_km(); while (agg_lane_length_km < deceleration_lane_length_km) { delta++; @@ -2412,12 +2419,19 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || - (xedge->IsHighway() && curr_edge->IsRampUse())) && - has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && - prev_edge->IsForkForward( - GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && - prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { - return true; + (xedge->IsHighway() && curr_edge->IsRampUse()))) { + + // If the ramp is actually a straight, announce the turn + if (is_nearly_straight(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { + return true; + } + + if (has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && + prev_edge->IsForkForward( + GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && + prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { + return true; + } } } diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc new file mode 100644 index 0000000000..edf47bdda6 --- /dev/null +++ b/test/gurka/test_forks.cc @@ -0,0 +1,136 @@ +#include "gurka.h" +#include "test.h" +#include + +#if !defined(VALHALLA_SOURCE_DIR) +#define VALHALLA_SOURCE_DIR +#endif + +using namespace valhalla; + +//======================================================================================= +class ForkTest : public ::testing::Test { +protected: + static gurka::map map; + static std::shared_ptr reader; + + static void SetUpTestSuite() { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + A-------B--------------------C----------------------------------------------------------D + E + F + )"; + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // BC is three lanes because it "grew" a deceleration lane. length = 200 m + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // now back to two lanes + {"CD", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // this ramp forms a small angle ~15 deg + {"CE", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}, + {"EF", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + } +}; + +std::shared_ptr ForkTest::reader; +gurka::map ForkTest::map = {}; + +//------------------------------------------------------------------ +TEST_F(ForkTest, test) { + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "auto"); + + // TODO: should not announce stay left since we detect a deceleration lane + for (const auto& leg : result.directions().routes(0).legs()) { + for (const auto& maneuver : leg.maneuver()) { + std::cout << maneuver.text_instruction() << "\n"; + } + } +} + +//======================================================================================= +class ForkTest2 : public ::testing::Test { +protected: + static gurka::map map; + static std::shared_ptr reader; + + static void SetUpTestSuite() { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + A-------B--------------------C----------------------------------------------------------D + E + F + )"; + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // BC is three lanes because it "grew" a deceleration lane. length = 200 m + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // now back to two lanes - but wait this is a different highway. ramp forms a small angle ~15 + // deg + {"CD", + {{"highway", "motorway_link"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D2"}, + {"maxspeed", "120"}}}, + // this is the continuation of D4 + {"CE", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}, + {"EF", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + } +}; + +std::shared_ptr ForkTest2::reader; +gurka::map ForkTest2::map = {}; + +//------------------------------------------------------------------ +TEST_F(ForkTest2, test) { + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "auto"); + + // TODO: since the "ramp" goes straight, announce the "stay right" + for (const auto& leg : result.directions().routes(0).legs()) { + for (const auto& maneuver : leg.maneuver()) { + std::cout << maneuver.text_instruction() << "\n"; + } + } +} From d2a4027059ecd26a600945c5250d8d5062c8164e Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Mon, 1 Nov 2021 16:59:25 -0600 Subject: [PATCH 10/23] test updates --- test/instructions.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/instructions.cc b/test/instructions.cc index 0a0b6af321..b6971e0fa7 100644 --- a/test/instructions.cc +++ b/test/instructions.cc @@ -311,16 +311,16 @@ TEST(Instructions, validate_exit_instructions) { test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/exit_left_driving_side_right.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Take the I 66 East exit on the left toward Washington.", "", - "Take the Interstate 66 East exit on the left.", - "Take the Interstate 66 East exit on the left toward Washington.", ""); + "Keep left to take I 66 East toward Washington.", "", + "Keep left to take Interstate 66 East.", + "Keep left to take Interstate 66 East toward Washington.", ""); // Test exit left on left driving side test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/exit_left_driving_side_left.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Take exit 8 onto A120(W)|A120(W).", "", "Take exit 8.", - "Take exit 8 onto A120(W)|A120(W).", ""); + "Keep left to take exit 8 onto A120(W)|A120(W).", "", "Keep left to take exit 8.", + "Keep left to take exit 8 onto A120(W)|A120(W).", ""); expected_maneuvers_size = 4; // Test exit non-motorway in PA @@ -540,10 +540,10 @@ TEST(Instructions, validate_obvious_maneuver_instructions) { test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/obvious_maneuver_short_continue.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Take the PA 39 West/Hersheypark Drive exit toward Attractions.", "", - "Take the Pennsylvania 39 West exit.", - "Take the Pennsylvania 39 West, Hersheypark Drive exit toward Attractions.", - "Continue for a half mile."); + "Keep right to take PA 39 West/Hersheypark Drive toward Attractions.", "", + "Keep right to take Pennsylvania 39 West.", + "Keep right to take Pennsylvania 39 West, Hersheypark Drive toward Attractions.", + ""); } } // namespace From 54de0e92876f2f92e8700f57599203786e288aa3 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Mon, 1 Nov 2021 21:32:03 -0600 Subject: [PATCH 11/23] tighter, undoing straight nonsense --- src/odin/maneuversbuilder.cc | 30 ++++++++++++++++++++++-------- test/instructions.cc | 19 +++++++++---------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index bedcf9d4c0..3d75a5bc1c 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2389,19 +2389,32 @@ bool ManeuversBuilder::IsFork(int node_index, // an exit than a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - float kt = prev_at_delta->speed(); - float deceleration_lane_length_km = - get_deceleration_lane_length(prev_at_delta->default_speed()); + float standard_deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); + float tol = 0.25 * standard_deceleration_lane_length_km; float agg_lane_length_km = prev_at_delta->length_km(); - while (agg_lane_length_km < deceleration_lane_length_km) { + + // iterate backwards until: + // 1) the extra lane goes away, or + // 2) we've exceeded a reasonable length for a deceleration lane + while (true) { delta++; prev_at_delta = trip_path->GetPrevEdge(node_index, delta); if (!prev_at_delta) break; + + bool extra_lane = (prev_at_delta->lane_count() == curr_lane_count + 1); + if (!extra_lane) + break; + + // aggregate (possible decel lane) length agg_lane_length_km += prev_at_delta->length_km(); + if (agg_lane_length_km < standard_deceleration_lane_length_km + tol) + break; } - if (prev_at_delta && (prev_at_delta->lane_count() == curr_lane_count)) { + // see if we're within 25% tolerance of a standard deceleration lane length + if ((agg_lane_length_km < standard_deceleration_lane_length_km + tol) && + (agg_lane_length_km > standard_deceleration_lane_length_km - tol)) { return false; } } @@ -2422,9 +2435,10 @@ bool ManeuversBuilder::IsFork(int node_index, (xedge->IsHighway() && curr_edge->IsRampUse()))) { // If the ramp is actually a straight, announce the turn - if (is_nearly_straight(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { - return true; - } + // needs work +// if (is_nearly_straight(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { +// return true; +// } if (has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && prev_edge->IsForkForward( diff --git a/test/instructions.cc b/test/instructions.cc index b6971e0fa7..249c9cf2be 100644 --- a/test/instructions.cc +++ b/test/instructions.cc @@ -311,17 +311,16 @@ TEST(Instructions, validate_exit_instructions) { test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/exit_left_driving_side_right.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Keep left to take I 66 East toward Washington.", "", - "Keep left to take Interstate 66 East.", - "Keep left to take Interstate 66 East toward Washington.", ""); + "Take the I 66 East exit on the left toward Washington.", "", + "Take the Interstate 66 East exit on the left.", + "Take the Interstate 66 East exit on the left toward Washington.", ""); // Test exit left on left driving side test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/exit_left_driving_side_left.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Keep left to take exit 8 onto A120(W)|A120(W).", "", "Keep left to take exit 8.", - "Keep left to take exit 8 onto A120(W)|A120(W).", ""); - + "Take exit 8 onto A120(W)|A120(W).", "", "Take exit 8.", + "Take exit 8 onto A120(W)|A120(W).", ""); expected_maneuvers_size = 4; // Test exit non-motorway in PA test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/exit_right_nonmotorway_pa.pbf"}, @@ -540,10 +539,10 @@ TEST(Instructions, validate_obvious_maneuver_instructions) { test_instructions({VALHALLA_SOURCE_DIR "test/pinpoints/instructions/obvious_maneuver_short_continue.pbf"}, expected_routes_size, expected_legs_size, expected_maneuvers_size, maneuver_index, - "Keep right to take PA 39 West/Hersheypark Drive toward Attractions.", "", - "Keep right to take Pennsylvania 39 West.", - "Keep right to take Pennsylvania 39 West, Hersheypark Drive toward Attractions.", - ""); + "Take the PA 39 West/Hersheypark Drive exit toward Attractions.", "", + "Take the Pennsylvania 39 West exit.", + "Take the Pennsylvania 39 West, Hersheypark Drive exit toward Attractions.", + "Continue for a half mile."); } } // namespace From c9829475c4abc28229a34f7b0e5c61c137bc3b94 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 08:20:56 -0600 Subject: [PATCH 12/23] format --- src/odin/maneuversbuilder.cc | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 3d75a5bc1c..03d7a13800 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2258,11 +2258,6 @@ float get_deceleration_lane_length(float speed_kph) { return 0.122; } -// This is a new idea... see usage. -bool is_nearly_straight(uint32_t turn_degree) { - return ((turn_degree > 357) || (turn_degree < 3)); -} - bool ManeuversBuilder::IsFork(int node_index, EnhancedTripLeg_Edge* prev_edge, EnhancedTripLeg_Edge* curr_edge) const { @@ -2389,7 +2384,8 @@ bool ManeuversBuilder::IsFork(int node_index, // an exit than a highway bifurcation. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - float standard_deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); + float standard_deceleration_lane_length_km = + get_deceleration_lane_length(prev_at_delta->default_speed()); float tol = 0.25 * standard_deceleration_lane_length_km; float agg_lane_length_km = prev_at_delta->length_km(); @@ -2433,13 +2429,6 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || (xedge->IsHighway() && curr_edge->IsRampUse()))) { - - // If the ramp is actually a straight, announce the turn - // needs work -// if (is_nearly_straight(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { -// return true; -// } - if (has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && prev_edge->IsForkForward( GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && From 5aafb68c9bc2d5c0a16042cf3739cc4c4c7242a6 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 12:45:02 -0600 Subject: [PATCH 13/23] more testing --- src/odin/maneuversbuilder.cc | 13 +- test/gurka/test_forks.cc | 327 +++++++++++++++++++++++------------ 2 files changed, 218 insertions(+), 122 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 03d7a13800..699a37d274 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2428,13 +2428,12 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || - (xedge->IsHighway() && curr_edge->IsRampUse()))) { - if (has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && - prev_edge->IsForkForward( - GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && - prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { - return true; - } + (xedge->IsHighway() && curr_edge->IsRampUse())) && + has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && + prev_edge->IsForkForward( + GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && + prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { + return true; } } diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc index edf47bdda6..5b0946f394 100644 --- a/test/gurka/test_forks.cc +++ b/test/gurka/test_forks.cc @@ -8,129 +8,226 @@ using namespace valhalla; -//======================================================================================= -class ForkTest : public ::testing::Test { -protected: - static gurka::map map; - static std::shared_ptr reader; - - static void SetUpTestSuite() { - constexpr double gridsize = 10; - - const std::string ascii_map = R"( - A-------B--------------------C----------------------------------------------------------D - E - F - )"; - const gurka::ways ways = - {// we start out with two lanes - {"AB", - {{"highway", "motorway"}, - {"oneway", "yes"}, - {"lanes", "2"}, - {"name", "A92"}, - {"maxspeed", "120"}}}, - // BC is three lanes because it "grew" a deceleration lane. length = 200 m - {"BC", - {{"highway", "motorway"}, - {"oneway", "yes"}, - {"lanes", "3"}, - {"name", "A92"}, - {"maxspeed", "120"}}}, - // now back to two lanes - {"CD", - {{"highway", "motorway"}, - {"oneway", "yes"}, - {"lanes", "2"}, - {"name", "A92"}, - {"maxspeed", "120"}}}, - // this ramp forms a small angle ~15 deg - {"CE", - {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}, - {"EF", - {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}}; - - const gurka::nodes nodes; - const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); - map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); - reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); - } -}; - -std::shared_ptr ForkTest::reader; -gurka::map ForkTest::map = {}; //------------------------------------------------------------------ -TEST_F(ForkTest, test) { +// We detect the deceleration lane and do not call out "Keep left to stay on A92." +//------------------------------------------------------------------ +TEST(ForkTest, StandardDecelerationLaneDontCallKeepLeft) { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + A-------B--------------------C----------------------------------------------------------D + E + F + )"; + + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // BC is three lanes because it "grew" a deceleration lane. length = 200 m + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // now back to two lanes + {"CD", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "A92"}, + {"maxspeed", "120"}}}, + // this ramp forms a small angle ~15 deg + {"CE", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}, + {"EF", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "A92 link"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "auto"); - // TODO: should not announce stay left since we detect a deceleration lane - for (const auto& leg : result.directions().routes(0).legs()) { - for (const auto& maneuver : leg.maneuver()) { - std::cout << maneuver.text_instruction() << "\n"; - } - } + EXPECT_EQ(result.directions().routes_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 2); + + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 0, + "Drive east on A92.", + "Drive east. Then, in 900 meters, You will arrive at your destination.", + "", + "Drive east on A92. Then, in 900 meters, You will arrive at your destination.", + "Continue for 900 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 1, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); +} + + +//------------------------------------------------------------------ +// The deceleration lane is too short. Have to call out "Keep right". +//------------------------------------------------------------------ +TEST(ForkTest, DecelerationLaneTooShort) { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + A-------B------------C----------------------------------------------------------D + E + F + )"; + + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // BC just "grew" a third lane. length = 130 m. + // This is too short to be considered a standard deceleration lane. + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // now back to two lanes - but wait this is a different highway. + // turn angle is ~15 deg. + {"CD", + {{"highway", "motorway_link"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D2"}, + {"maxspeed", "120"}}}, + // this is the continuation of D4 + {"CE", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}, + {"EF", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "auto"); + + EXPECT_EQ(result.directions().routes_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 3); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 0, + "Drive east on D4.", + "Drive east. Then Keep right to stay on D4.", + "", + "Drive east on D4. Then Keep right to stay on D4.", + "Continue for 200 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 1, + "Keep right to stay on D4.", + "", + "Keep right to stay on D4.", + "Keep right to stay on D4. Then You will arrive at your destination.", + "Continue for 100 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 2, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); } -//======================================================================================= -class ForkTest2 : public ::testing::Test { -protected: - static gurka::map map; - static std::shared_ptr reader; - - static void SetUpTestSuite() { - constexpr double gridsize = 10; - - const std::string ascii_map = R"( - A-------B--------------------C----------------------------------------------------------D - E - F - )"; - const gurka::ways ways = - {// we start out with two lanes - {"AB", - {{"highway", "motorway"}, - {"oneway", "yes"}, - {"lanes", "2"}, - {"name", "D4"}, - {"maxspeed", "120"}}}, - // BC is three lanes because it "grew" a deceleration lane. length = 200 m - {"BC", - {{"highway", "motorway"}, - {"oneway", "yes"}, - {"lanes", "3"}, - {"name", "D4"}, - {"maxspeed", "120"}}}, - // now back to two lanes - but wait this is a different highway. ramp forms a small angle ~15 - // deg - {"CD", - {{"highway", "motorway_link"}, - {"oneway", "yes"}, - {"lanes", "2"}, - {"name", "D2"}, - {"maxspeed", "120"}}}, - // this is the continuation of D4 - {"CE", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}, - {"EF", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}}; - - const gurka::nodes nodes; - const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); - map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); - reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); - } -}; - -std::shared_ptr ForkTest2::reader; -gurka::map ForkTest2::map = {}; //------------------------------------------------------------------ -TEST_F(ForkTest2, test) { +// The deceleration lane is too long. Have to call out "Keep right". +//------------------------------------------------------------------ +TEST(ForkTest, DecelerationLaneTooLong) { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + A-------B-----------------------------C----------------------------------------------------------D + E + F + )"; + + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // BC just "grew" a third lane. length = 300 m. + // This is too long to be considered a standard deceleration lane. + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // now back to two lanes - but wait this is a different highway. + // turn angle is ~15 deg. + {"CD", + {{"highway", "motorway_link"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D2"}, + {"maxspeed", "120"}}}, + // this is the continuation of D4 + {"CE", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}, + {"EF", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "auto"); - // TODO: since the "ramp" goes straight, announce the "stay right" - for (const auto& leg : result.directions().routes(0).legs()) { - for (const auto& maneuver : leg.maneuver()) { - std::cout << maneuver.text_instruction() << "\n"; - } - } + EXPECT_EQ(result.directions().routes_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 3); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 0, + "Drive east on D4.", + "Drive east. Then Keep right to stay on D4.", + "", + "Drive east on D4. Then Keep right to stay on D4.", + "Continue for 400 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 1, + "Keep right to stay on D4.", + "", + "Keep right to stay on D4.", + "Keep right to stay on D4. Then You will arrive at your destination.", + "Continue for 100 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 2, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); } From ff1f794be13799d69d18a4afb165a3e1d736ec46 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 13:11:22 -0600 Subject: [PATCH 14/23] touch ups --- src/odin/maneuversbuilder.cc | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 699a37d274..4b0743dcc8 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2376,12 +2376,7 @@ bool ManeuversBuilder::IsFork(int node_index, if ((curr_lane_count > 1) && (prev_lane_count == curr_lane_count + 1) && (xedge->use() == TripLeg_Use_kRampUse)) { - // Determine if this lane split is a deceleration lane forking onto an - // exit. A deceleration lane will have a predictable length. Iterate previous - // edges until we've exceeded the length of a deceleration lane. If the - // highway at that point has gone down a lane, then it is likely that - // prev-edge is a deceleration lane - which means this is more likely - // an exit than a highway bifurcation. + // Determine if this lane split is a deceleration lane forking onto an exit. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); float standard_deceleration_lane_length_km = @@ -2398,8 +2393,9 @@ bool ManeuversBuilder::IsFork(int node_index, if (!prev_at_delta) break; - bool extra_lane = (prev_at_delta->lane_count() == curr_lane_count + 1); - if (!extra_lane) + // See if the extra lane continues. If it goes away, break; + bool extra_lane_continues = (prev_at_delta->lane_count() == curr_lane_count + 1); + if (!extra_lane_continues) break; // aggregate (possible decel lane) length @@ -2408,7 +2404,9 @@ bool ManeuversBuilder::IsFork(int node_index, break; } - // see if we're within 25% tolerance of a standard deceleration lane length + // see if we're within 25% tolerance of a standard deceleration lane length. + // if so, we consider this fork a deceleration lane and we don not consider + // this a lane bifurcation. if ((agg_lane_length_km < standard_deceleration_lane_length_km + tol) && (agg_lane_length_km > standard_deceleration_lane_length_km - tol)) { return false; @@ -2429,7 +2427,7 @@ bool ManeuversBuilder::IsFork(int node_index, if (prev_edge->IsHighway() && ((curr_edge->IsHighway() && (xedge->use() == TripLeg_Use_kRampUse)) || (xedge->IsHighway() && curr_edge->IsRampUse())) && - has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && + has_lane_bifurcation(trip_path_, node_index, prev_edge, curr_edge, xedge) && prev_edge->IsForkForward( GetTurnDegree(prev_edge->end_heading(), curr_edge->begin_heading())) && prev_edge->IsForkForward(GetTurnDegree(prev_edge->end_heading(), xedge->begin_heading()))) { From ab2417838cdf884fb3ce78d6a233721c30468cc4 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 13:23:52 -0600 Subject: [PATCH 15/23] format for new test logic --- test/gurka/test_forks.cc | 116 +++++++++++++++++---------------------- 1 file changed, 50 insertions(+), 66 deletions(-) diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc index 5b0946f394..b789d61530 100644 --- a/test/gurka/test_forks.cc +++ b/test/gurka/test_forks.cc @@ -8,7 +8,6 @@ using namespace valhalla; - //------------------------------------------------------------------ // We detect the deceleration lane and do not call out "Keep left to stay on A92." //------------------------------------------------------------------ @@ -52,7 +51,8 @@ TEST(ForkTest, StandardDecelerationLaneDontCallKeepLeft) { const gurka::nodes nodes; const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); - std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + std::shared_ptr reader = + test::make_clean_graphreader(map.config.get_child("mjolnir")); auto result = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "auto"); @@ -60,25 +60,20 @@ TEST(ForkTest, StandardDecelerationLaneDontCallKeepLeft) { EXPECT_EQ(result.directions().routes(0).legs_size(), 1); EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 2); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 0, - "Drive east on A92.", - "Drive east. Then, in 900 meters, You will arrive at your destination.", - "", - "Drive east on A92. Then, in 900 meters, You will arrive at your destination.", - "Continue for 900 meters."); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 1, - "You have arrived at your destination.", - "", - "You will arrive at your destination.", - "You have arrived at your destination.", - ""); + gurka::assert::raw::expect_instructions_at_maneuver_index( + result, 0, "Drive east on A92.", + "Drive east. Then, in 900 meters, You will arrive at your destination.", "", + "Drive east on A92. Then, in 900 meters, You will arrive at your destination.", + "Continue for 900 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 1, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); } - //------------------------------------------------------------------ // The deceleration lane is too short. Have to call out "Keep right". //------------------------------------------------------------------ @@ -122,7 +117,8 @@ TEST(ForkTest, DecelerationLaneTooShort) { const gurka::nodes nodes; const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); - std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + std::shared_ptr reader = + test::make_clean_graphreader(map.config.get_child("mjolnir")); auto result = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "auto"); @@ -131,31 +127,24 @@ TEST(ForkTest, DecelerationLaneTooShort) { EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 3); gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 0, - "Drive east on D4.", - "Drive east. Then Keep right to stay on D4.", - "", - "Drive east on D4. Then Keep right to stay on D4.", - "Continue for 200 meters."); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 1, - "Keep right to stay on D4.", - "", - "Keep right to stay on D4.", - "Keep right to stay on D4. Then You will arrive at your destination.", - "Continue for 100 meters."); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 2, - "You have arrived at your destination.", - "", - "You will arrive at your destination.", - "You have arrived at your destination.", - ""); + expect_instructions_at_maneuver_index(result, 0, "Drive east on D4.", + "Drive east. Then Keep right to stay on D4.", "", + "Drive east on D4. Then Keep right to stay on D4.", + "Continue for 200 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index( + result, 1, "Keep right to stay on D4.", "", "Keep right to stay on D4.", + "Keep right to stay on D4. Then You will arrive at your destination.", + "Continue for 100 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 2, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); } - //------------------------------------------------------------------ // The deceleration lane is too long. Have to call out "Keep right". //------------------------------------------------------------------ @@ -199,7 +188,8 @@ TEST(ForkTest, DecelerationLaneTooLong) { const gurka::nodes nodes; const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); - std::shared_ptr reader = test::make_clean_graphreader(map.config.get_child("mjolnir")); + std::shared_ptr reader = + test::make_clean_graphreader(map.config.get_child("mjolnir")); auto result = gurka::do_action(valhalla::Options::route, map, {"A", "F"}, "auto"); @@ -208,26 +198,20 @@ TEST(ForkTest, DecelerationLaneTooLong) { EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 3); gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 0, - "Drive east on D4.", - "Drive east. Then Keep right to stay on D4.", - "", - "Drive east on D4. Then Keep right to stay on D4.", - "Continue for 400 meters."); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 1, - "Keep right to stay on D4.", - "", - "Keep right to stay on D4.", - "Keep right to stay on D4. Then You will arrive at your destination.", - "Continue for 100 meters."); - - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 2, - "You have arrived at your destination.", - "", - "You will arrive at your destination.", - "You have arrived at your destination.", - ""); + expect_instructions_at_maneuver_index(result, 0, "Drive east on D4.", + "Drive east. Then Keep right to stay on D4.", "", + "Drive east on D4. Then Keep right to stay on D4.", + "Continue for 400 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index( + result, 1, "Keep right to stay on D4.", "", "Keep right to stay on D4.", + "Keep right to stay on D4. Then You will arrive at your destination.", + "Continue for 100 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 2, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); } From 09d25a6de2b0876eded79b0edbadac76400e8bd5 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 14:35:43 -0600 Subject: [PATCH 16/23] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b457a18b7b..df67cded37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * FIXED: Fix precision losses while encoding-decoding distance parameter in openlr [#3374](https://github.com/valhalla/valhalla/pull/3374) * FIXED: Fix bearing calculation for openlr records [#3379](https://github.com/valhalla/valhalla/pull/3379) * FIXED: Some refactoring that was proposed for the PR 3379 [3381](https://github.com/valhalla/valhalla/pull/3381) + * FIXED: Avoid calling out "keep left/right" when passing an exit [3349](https://github.com/valhalla/valhalla/pull/3349) * **Enhancement** * CHANGED: Pronunciation for names and destinations [#3132](https://github.com/valhalla/valhalla/pull/3132) From 3a7043628ef265274f8472c877770ff7a640d71e Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 2 Nov 2021 16:59:05 -0600 Subject: [PATCH 17/23] comment --- src/odin/maneuversbuilder.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 4b0743dcc8..aef9c8a7a0 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2405,8 +2405,8 @@ bool ManeuversBuilder::IsFork(int node_index, } // see if we're within 25% tolerance of a standard deceleration lane length. - // if so, we consider this fork a deceleration lane and we don not consider - // this a lane bifurcation. + // if so, we consider this fork as preceded by a deceleration lane leading to + // a fork/exit and we do not consider this a lane bifurcation. if ((agg_lane_length_km < standard_deceleration_lane_length_km + tol) && (agg_lane_length_km > standard_deceleration_lane_length_km - tol)) { return false; From 39ff82189b18f093a174a87fe8b1650100a771bb Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 4 Nov 2021 08:45:33 -0600 Subject: [PATCH 18/23] Some fixes and new tests to back up fixes --- src/odin/maneuversbuilder.cc | 54 +++++++------ test/gurka/test_forks.cc | 146 ++++++++++++++++++++++++++++++++++- 2 files changed, 172 insertions(+), 28 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index aef9c8a7a0..300c254cc5 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2236,26 +2236,22 @@ bool ManeuversBuilder::IsMergeManeuverType(Maneuver& maneuver, return false; } -// return the length in km for a deceleration lane as a function of the road's speed. -// -// speed (mph) | decel lane length -// -----------------|-------------------- -// >70 | 700 ft -// 60-70 | 600 ft -// 50-60 | 500 ft -// <50 | 400 ft +// Return the length in km for a deceleration lane as a function of the road's speed. +// This data comes from a few sources that were similar. After studying their data +// I realized they were both (nearly) linear, so this routine boils both sources +// down to an equation for a line (with bounds on x). Note that deceleration lane lengths +// can vary, so I advise using a tolerance, e.g., "is this lane within 10% of the expected +// deceleration lane length?". float get_deceleration_lane_length(float speed_kph) { - if (speed_kph > 112) { - return 0.213; + if (speed_kph > 110) { + return 0.204; } - if (speed_kph > 96) { - return 0.183; - } - if (speed_kph > 80) { - return 0.152; + + if (speed_kph < 30) { + return 0.060; } - return 0.122; + return 0.0022254 * speed_kph - 0.0472; } bool ManeuversBuilder::IsFork(int node_index, @@ -2379,9 +2375,10 @@ bool ManeuversBuilder::IsFork(int node_index, // Determine if this lane split is a deceleration lane forking onto an exit. int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); + auto orig_prev_at_delta_lane_count = prev_at_delta->lane_count(); float standard_deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); - float tol = 0.25 * standard_deceleration_lane_length_km; + float tol = 0.1 * standard_deceleration_lane_length_km; float agg_lane_length_km = prev_at_delta->length_km(); // iterate backwards until: @@ -2390,21 +2387,30 @@ bool ManeuversBuilder::IsFork(int node_index, while (true) { delta++; prev_at_delta = trip_path->GetPrevEdge(node_index, delta); - if (!prev_at_delta) + // prev_at_delta is null when we cannot walk backwards any further + // (e.g., we've hit the beginning of the route). If this occurs + // set agg_lane_length_km to 0.0 to ensure we do not consider + // this a deceleration lane. + if (!prev_at_delta) { + agg_lane_length_km = 0.0; break; + } - // See if the extra lane continues. If it goes away, break; - bool extra_lane_continues = (prev_at_delta->lane_count() == curr_lane_count + 1); - if (!extra_lane_continues) + // See if the extra lane goes away. + auto prev_at_delta_lane_count = prev_at_delta->lane_count(); + bool extra_lane_goes_away = (prev_at_delta_lane_count < orig_prev_at_delta_lane_count); + if (extra_lane_goes_away) break; - // aggregate (possible decel lane) length + // aggregate (possible deceleration lane) length. agg_lane_length_km += prev_at_delta->length_km(); - if (agg_lane_length_km < standard_deceleration_lane_length_km + tol) + + // if we've exceeded the standard length for a deceleration lane, we're done. + if (agg_lane_length_km > standard_deceleration_lane_length_km + tol) break; } - // see if we're within 25% tolerance of a standard deceleration lane length. + // see if we're within tolerance of a standard deceleration lane length. // if so, we consider this fork as preceded by a deceleration lane leading to // a fork/exit and we do not consider this a lane bifurcation. if ((agg_lane_length_km < standard_deceleration_lane_length_km + tol) && diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc index b789d61530..03955c833c 100644 --- a/test/gurka/test_forks.cc +++ b/test/gurka/test_forks.cc @@ -74,6 +74,75 @@ TEST(ForkTest, StandardDecelerationLaneDontCallKeepLeft) { ""); } +//------------------------------------------------------------------ +// We detect the deceleration lane and do not call out "Keep right to stay on M1." +//------------------------------------------------------------------ +TEST(ForkTest, StandardDecelerationLaneDontCallKeepRight) { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + F + E + A-------B--------------------C----------------------------------------------------------D + )"; + + const gurka::ways ways = + {// we start out with two lanes + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "M1"}, + {"driving_side", "left"}, + {"maxspeed", "120"}}}, + // BC is three lanes because it "grew" a deceleration lane. length = 200 m + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "M1"}, + {"driving_side", "left"}, + {"maxspeed", "120"}}}, + // now back to two lanes + {"CD", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "M1"}, + {"driving_side", "left"}, + {"maxspeed", "120"}}}, + // this ramp forms a small angle ~15 deg + {"CE", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}, {"driving_side", "left"}}}, + {"EF", + {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}, {"driving_side", "left"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + std::shared_ptr reader = + test::make_clean_graphreader(map.config.get_child("mjolnir")); + + auto result = gurka::do_action(valhalla::Options::route, map, {"A", "D"}, "auto"); + + EXPECT_EQ(result.directions().routes_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 2); + + gurka::assert::raw::expect_instructions_at_maneuver_index( + result, 0, "Drive east on M1.", + "Drive east. Then, in 900 meters, You will arrive at your destination.", "", + "Drive east on M1. Then, in 900 meters, You will arrive at your destination.", + "Continue for 900 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 1, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); +} + //------------------------------------------------------------------ // The deceleration lane is too short. Have to call out "Keep right". //------------------------------------------------------------------ @@ -81,9 +150,9 @@ TEST(ForkTest, DecelerationLaneTooShort) { constexpr double gridsize = 10; const std::string ascii_map = R"( - A-------B------------C----------------------------------------------------------D - E - F + A---------------------------B------------C----------------------------------------------------------D + E + F )"; const gurka::ways ways = @@ -130,7 +199,7 @@ TEST(ForkTest, DecelerationLaneTooShort) { expect_instructions_at_maneuver_index(result, 0, "Drive east on D4.", "Drive east. Then Keep right to stay on D4.", "", "Drive east on D4. Then Keep right to stay on D4.", - "Continue for 200 meters."); + "Continue for 400 meters."); gurka::assert::raw::expect_instructions_at_maneuver_index( result, 1, "Keep right to stay on D4.", "", "Keep right to stay on D4.", @@ -215,3 +284,72 @@ TEST(ForkTest, DecelerationLaneTooLong) { "You have arrived at your destination.", ""); } + +//------------------------------------------------------------------ +// The route starts on what might be a deceleration lane. However, +// the deceleration-lane-detection logic cannot walk backwards far +// enough to make the determination. Therefore the logic calls out +// the intermediate maneuver to "keep left...". +//------------------------------------------------------------------ +TEST(ForkTest, RouteStartsOnPossibleDecelLane) { + constexpr double gridsize = 10; + + const std::string ascii_map = R"( + B------------------C----------------------------------------------------------D + E + F + )"; + + const gurka::ways ways = + { + // BC is three lanes. it is long enough to be considered a deceleration + // lane, but the route starts at B so we cannot assess with certainty + // that it is one. + {"BC", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // now back to two lanes - but wait this is a different highway. + // turn angle is ~15 deg. + {"CD", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + {"CE", {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}}}, + {"EF", {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}}}}; + + const gurka::nodes nodes; + const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); + gurka::map map = gurka::buildtiles(layout, ways, nodes, {}, "test/data/ForkTest", {}); + std::shared_ptr reader = + test::make_clean_graphreader(map.config.get_child("mjolnir")); + + auto result = gurka::do_action(valhalla::Options::route, map, {"B", "D"}, "auto"); + + EXPECT_EQ(result.directions().routes_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs_size(), 1); + EXPECT_EQ(result.directions().routes(0).legs(0).maneuver_size(), 3); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 0, "Drive east on D4.", + "Drive east. Then Keep left to stay on D4.", "", + "Drive east on D4. Then Keep left to stay on D4.", + "Continue for 200 meters."); + + gurka::assert::raw:: + expect_instructions_at_maneuver_index(result, 1, "Keep left to stay on D4.", + "", "Keep left to stay on D4.", + "Keep left to stay on D4.", + "Continue for 600 meters."); + + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 2, + "You have arrived at your destination.", + "", + "You will arrive at your destination.", + "You have arrived at your destination.", + ""); +} From 1a8ca2b7cec7eadb2015e8c5760301c34a973804 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 4 Nov 2021 08:49:54 -0600 Subject: [PATCH 19/23] format --- test/gurka/test_forks.cc | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc index 03955c833c..43a7133411 100644 --- a/test/gurka/test_forks.cc +++ b/test/gurka/test_forks.cc @@ -113,9 +113,17 @@ TEST(ForkTest, StandardDecelerationLaneDontCallKeepRight) { {"maxspeed", "120"}}}, // this ramp forms a small angle ~15 deg {"CE", - {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}, {"driving_side", "left"}}}, + {{"highway", "motorway_link"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D5"}, + {"driving_side", "left"}}}, {"EF", - {{"highway", "motorway_link"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D5"}, {"driving_side", "left"}}}}; + {{"highway", "motorway_link"}, + {"oneway", "yes"}, + {"lanes", "2"}, + {"name", "D5"}, + {"driving_side", "left"}}}}; const gurka::nodes nodes; const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize); @@ -301,8 +309,7 @@ TEST(ForkTest, RouteStartsOnPossibleDecelLane) { )"; const gurka::ways ways = - { - // BC is three lanes. it is long enough to be considered a deceleration + {// BC is three lanes. it is long enough to be considered a deceleration // lane, but the route starts at B so we cannot assess with certainty // that it is one. {"BC", @@ -340,11 +347,10 @@ TEST(ForkTest, RouteStartsOnPossibleDecelLane) { "Drive east on D4. Then Keep left to stay on D4.", "Continue for 200 meters."); - gurka::assert::raw:: - expect_instructions_at_maneuver_index(result, 1, "Keep left to stay on D4.", - "", "Keep left to stay on D4.", - "Keep left to stay on D4.", - "Continue for 600 meters."); + gurka::assert::raw::expect_instructions_at_maneuver_index(result, 1, "Keep left to stay on D4.", "", + "Keep left to stay on D4.", + "Keep left to stay on D4.", + "Continue for 600 meters."); gurka::assert::raw::expect_instructions_at_maneuver_index(result, 2, "You have arrived at your destination.", From 9768af370d79ac801000e2fb9aadeb149a2963d8 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Thu, 4 Nov 2021 09:04:34 -0600 Subject: [PATCH 20/23] fixing test case --- test/gurka/test_forks.cc | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/gurka/test_forks.cc b/test/gurka/test_forks.cc index 43a7133411..f94ad57fc7 100644 --- a/test/gurka/test_forks.cc +++ b/test/gurka/test_forks.cc @@ -223,27 +223,34 @@ TEST(ForkTest, DecelerationLaneTooShort) { } //------------------------------------------------------------------ -// The deceleration lane is too long. Have to call out "Keep right". +// The deceleration lane is too long (so not really a deceleration lane). +// Have to call out "Keep right". //------------------------------------------------------------------ TEST(ForkTest, DecelerationLaneTooLong) { constexpr double gridsize = 10; const std::string ascii_map = R"( - A-------B-----------------------------C----------------------------------------------------------D - E - F + Z------------A-----------------B-----------------C----------------D + E + F )"; const gurka::ways ways = {// we start out with two lanes - {"AB", + {"ZA", {{"highway", "motorway"}, {"oneway", "yes"}, {"lanes", "2"}, {"name", "D4"}, {"maxspeed", "120"}}}, - // BC just "grew" a third lane. length = 300 m. - // This is too long to be considered a standard deceleration lane. + // grow a lane + {"AB", + {{"highway", "motorway"}, + {"oneway", "yes"}, + {"lanes", "3"}, + {"name", "D4"}, + {"maxspeed", "120"}}}, + // AB & BC combined are too long to be considered a deceleration lane {"BC", {{"highway", "motorway"}, {"oneway", "yes"}, From dbc7a5f45c4fe1ecb858e67edff92842bed860bd Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Fri, 5 Nov 2021 08:02:50 -0600 Subject: [PATCH 21/23] setting tol to 30 --- src/odin/maneuversbuilder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 300c254cc5..21ea315127 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2378,7 +2378,7 @@ bool ManeuversBuilder::IsFork(int node_index, auto orig_prev_at_delta_lane_count = prev_at_delta->lane_count(); float standard_deceleration_lane_length_km = get_deceleration_lane_length(prev_at_delta->default_speed()); - float tol = 0.1 * standard_deceleration_lane_length_km; + float tol = 0.30 * standard_deceleration_lane_length_km; float agg_lane_length_km = prev_at_delta->length_km(); // iterate backwards until: From ef0074d6a6477cea00d2e4eb575fdb2256039adc Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Mon, 8 Nov 2021 13:15:36 -0700 Subject: [PATCH 22/23] dialing in the deceleration lane length equation --- src/odin/maneuversbuilder.cc | 49 +++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 21ea315127..88771492a2 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2236,22 +2236,35 @@ bool ManeuversBuilder::IsMergeManeuverType(Maneuver& maneuver, return false; } -// Return the length in km for a deceleration lane as a function of the road's speed. -// This data comes from a few sources that were similar. After studying their data -// I realized they were both (nearly) linear, so this routine boils both sources -// down to an equation for a line (with bounds on x). Note that deceleration lane lengths -// can vary, so I advise using a tolerance, e.g., "is this lane within 10% of the expected -// deceleration lane length?". -float get_deceleration_lane_length(float speed_kph) { - if (speed_kph > 110) { - return 0.204; +// Return the (min,max) length in km for a deceleration lane as a function of the road's +// speed. +// +// This equation started out as an approximation based on a couple research papers +// I found online. However, that equation wasn't quite catching faster-roadways with +// longer deceleration lanes. So I gave it a buffer and empirically derived the equation +// below by running ~100 long routes all over the world, grabbing the real world road +// speed (x) and deceleration lane lengths (y). I plotted all the gathered points and +// observed a the trend is linear. I then ran these points through a linear regression +// which resulted in the equation below. It is also important to mention that the +// deceleration lane length can vary quite a bit. Using the same data, the deceleration +// lane length can vary by as much as 40% compared to the value returned by this equation. +// However, a tolerance of 35% catches all but the extreme outliers. +std::pair get_deceleration_lane_length(float speed_kph) { + float length; + + // Below 80 kph I found that decel lanes are all roughly this length + if (speed_kph < 80) { + length = 0.1; + } else { + length = 0.00141994 * speed_kph + 0.03509388; } - if (speed_kph < 30) { - return 0.060; - } + // Empirically derived to catch all but the most extreme outliers. + constexpr float pct_tol = 0.35; + + float tol = length * pct_tol; - return 0.0022254 * speed_kph - 0.0472; + return std::make_pair(length - tol, length + tol); } bool ManeuversBuilder::IsFork(int node_index, @@ -2376,9 +2389,9 @@ bool ManeuversBuilder::IsFork(int node_index, int delta = 1; auto prev_at_delta = trip_path->GetPrevEdge(node_index, delta); auto orig_prev_at_delta_lane_count = prev_at_delta->lane_count(); - float standard_deceleration_lane_length_km = + float min_deceleration_lane_length_km, max_deceleration_lane_length_km; + std::tie(min_deceleration_lane_length_km, max_deceleration_lane_length_km) = get_deceleration_lane_length(prev_at_delta->default_speed()); - float tol = 0.30 * standard_deceleration_lane_length_km; float agg_lane_length_km = prev_at_delta->length_km(); // iterate backwards until: @@ -2406,15 +2419,15 @@ bool ManeuversBuilder::IsFork(int node_index, agg_lane_length_km += prev_at_delta->length_km(); // if we've exceeded the standard length for a deceleration lane, we're done. - if (agg_lane_length_km > standard_deceleration_lane_length_km + tol) + if (agg_lane_length_km > max_deceleration_lane_length_km) break; } // see if we're within tolerance of a standard deceleration lane length. // if so, we consider this fork as preceded by a deceleration lane leading to // a fork/exit and we do not consider this a lane bifurcation. - if ((agg_lane_length_km < standard_deceleration_lane_length_km + tol) && - (agg_lane_length_km > standard_deceleration_lane_length_km - tol)) { + if ((agg_lane_length_km < max_deceleration_lane_length_km) && + (agg_lane_length_km > min_deceleration_lane_length_km)) { return false; } } From 286f58445704646622321ddee5b4600a6f1b3367 Mon Sep 17 00:00:00 2001 From: Kevin Tatterson Date: Tue, 9 Nov 2021 08:17:39 -0700 Subject: [PATCH 23/23] comment --- src/odin/maneuversbuilder.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc index 88771492a2..149fac9c11 100644 --- a/src/odin/maneuversbuilder.cc +++ b/src/odin/maneuversbuilder.cc @@ -2244,7 +2244,7 @@ bool ManeuversBuilder::IsMergeManeuverType(Maneuver& maneuver, // longer deceleration lanes. So I gave it a buffer and empirically derived the equation // below by running ~100 long routes all over the world, grabbing the real world road // speed (x) and deceleration lane lengths (y). I plotted all the gathered points and -// observed a the trend is linear. I then ran these points through a linear regression +// observed a linear trend. I then ran these points through a linear regression // which resulted in the equation below. It is also important to mention that the // deceleration lane length can vary quite a bit. Using the same data, the deceleration // lane length can vary by as much as 40% compared to the value returned by this equation. @@ -2440,6 +2440,7 @@ bool ManeuversBuilder::IsFork(int node_index, (xedge_lane_count == post_split_min_count)) { return true; } + return false; };