Ensure u-turn exists in intersection view #6376

mjjbell · 2022-09-25T15:11:38Z

Context

The full graphic details are in #6218

The motivation for this PR is to unblock the debug build on very large (i.e. global) datasets. Currently they will fail on this assertion, so we are unable to reach issues further down the processing chain.

Issue

Due to some rather complex logic that tries to calculate intersection angles by looking further up the road, it's possible to return an intersection view that is missing a u-turn - something which is assumed to exist in later guidance calculations.

We apply a fix here by ensuring the u-turn is always included in the returned view.

Ideally we would refactor what looks like some quite brittle intersection code, but that requires:

better understanding of the guidance code
a large and complicated change

so this appears to be the better alternative in the short-term.

Tasklist

CHANGELOG.md entry (How to write a changelog entry)
update relevant Wiki pages
add tests (see testing documentation)
review
adjust for comments
cherry pick to release branch

Requirements / Relations

Fixes #6218

mjjbell · 2022-09-25T15:25:44Z

src/extractor/intersection/intersection_analysis.cpp

@@ -678,6 +679,7 @@ IntersectionView convertToIntersectionView(const util::NodeBasedDynamicGraph &gr
    BOOST_ASSERT(uturn.eid != SPECIAL_EDGEID);
    if (uturn.entry_allowed || allowed_uturns_number == 0)
    { // Add the true U-turn if it is allowed or no other U-turns found
+        BOOST_ASSERT(uturn.angle == 0.);


angle is the perceived angle between the incoming and outgoing edges at the intersection. It can differ from the real angle because it does smart stuff like look further ahead down the road to understand where we're actually heading.

For the u-turn incoming/outgoing pair along the same edge, any changes in angle will always match, so the difference remains 0.. This is the important assumption that by adding it here guarantees we always have a 0° turn in the intersection view.

mjjbell · 2022-09-25T15:27:07Z

src/extractor/intersection/intersection_analysis.cpp

+    if (no_uturn)
+    {
+        BOOST_ASSERT(!uturn.entry_allowed && allowed_uturns_number > 0);
+        BOOST_ASSERT(uturn.angle == 0.);


Same comment as above. The other assertion is because we only manually insert here if we didn't above.

SiarheiFedartsou · 2022-09-25T21:22:13Z

src/extractor/intersection/intersection_analysis.cpp

+    auto no_uturn = std::none_of(
+        pre_intersection_view.begin(),
+        pre_intersection_view.end(),
+        [&is_uturn](IntersectionViewDataWithAngle road) { return is_uturn(road.first.angle); });


nit. probably worth passing road by const reference: I.e. const auto& road

Due to some rather complex logic that tries to calculate intersection angles by looking further up the road, it's possible to return an intersection view that is missing a u-turn - something which is assumed to exist in later guidance calculations. We apply a fix here by ensuring the u-turn is always included in the returned view.

v5.27.0 - Changes from 5.26.0 - API: - ADDED: Add Flatbuffers support to NodeJS bindings. [Project-OSRM#6338](Project-OSRM#6338) - CHANGED: Add `data_version` field to responses of all services. [Project-OSRM#5387](Project-OSRM#5387) - FIXED: Use Boost.Beast to parse HTTP request. [Project-OSRM#6294](Project-OSRM#6294) - FIXED: Fix inefficient osrm-routed connection handling [Project-OSRM#6113](https://gihub.com/Project-OSRM/osrm-backend/pull/6113) - FIXED: Fix HTTP compression precedence [Project-OSRM#6113](Project-OSRM#6113) - NodeJS: - FIXED: Support `skip_waypoints` in Node bindings [Project-OSRM#6060](Project-OSRM#6060) - Misc: - ADDED: conanbuildinfo.json for easy reading of dependencies [Project-OSRM#6388](Project-OSRM#6388) - CHANGED: Improve performance of JSON rendering. Fix undefined behaviour in JSON numbers formatting. [Project-OSRM#6380](Project-OSRM#6380) - ADDED: Add timestamps for logs. [Project-OSRM#6375](Project-OSRM#6375) - CHANGED: Improve performance of map matching via getPathDistance optimization. [Project-OSRM#6378](Project-OSRM#6378) - CHANGED: Optimize RestrictionParser performance. [Project-OSRM#6344](Project-OSRM#6344) - ADDED: Support floats for speed value in traffic updates CSV. [Project-OSRM#6327](Project-OSRM#6327) - CHANGED: Use Lua 5.4 in Docker image. [Project-OSRM#6346](Project-OSRM#6346) - CHANGED: Remove redundant nullptr check. [Project-OSRM#6326](Project-OSRM#6326) - CHANGED: missing files list is included in exception message. [Project-OSRM#5360](Project-OSRM#5360) - CHANGED: Do not use deprecated Callback::Call overload in Node bindings. [Project-OSRM#6318](Project-OSRM#6318) - FIXED: Fix distance calculation consistency. [Project-OSRM#6315](Project-OSRM#6315) - FIXED: Fix performance issue after migration to sol2 3.3.0. [Project-OSRM#6304](Project-OSRM#6304) - CHANGED: Pass osm_node_ids by reference in osrm::updater::Updater class. [Project-OSRM#6298](Project-OSRM#6298) - FIXED: Fix bug with reading Set values from Lua scripts. [Project-OSRM#6285](Project-OSRM#6285) - FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [Project-OSRM#6296](Project-OSRM#6296) - FIXED: Internal refactoring of identifier types used in data facade [Project-OSRM#6044](Project-OSRM#6044) - CHANGED: Update docs to reflect recent build and dependency changes [Project-OSRM#6383](Project-OSRM#6383) - Build: - REMOVED: Get rid of Mason. [Project-OSRM#6387](Project-OSRM#6387) - CHANGED: Use clang-format from CI base image. [Project-OSRM#6391](Project-OSRM#6391) - ADDED: Build Node bindings on Windows. [Project-OSRM#6334](Project-OSRM#6334) - ADDED: Configure cross-compilation for Apple Silicon. [Project-OSRM#6360](Project-OSRM#6360) - CHANGED: Use apt-get to install Clang on CI. [Project-OSRM#6345](Project-OSRM#6345) - CHANGED: Fix TBB in case of Conan + NodeJS build. [Project-OSRM#6333](Project-OSRM#6333) - CHANGED: Migrate to modern TBB version. [Project-OSRM#6300](Project-OSRM#6300) - CHANGED: Enable performance-move-const-arg clang-tidy check. [Project-OSRM#6319](Project-OSRM#6319) - CHANGED: Use the latest node on CI. [Project-OSRM#6317](Project-OSRM#6317) - CHANGED: Migrate Windows CI to GitHub Actions. [Project-OSRM#6312](Project-OSRM#6312) - ADDED: Add smoke test for Docker image. [Project-OSRM#6313](Project-OSRM#6313) - CHANGED: Update libosmium to version 2.18.0. [Project-OSRM#6303](Project-OSRM#6303) - CHANGED: Remove EXACT from find_package if using Conan. [Project-OSRM#6299](Project-OSRM#6299) - CHANGED: Configure Undefined Behaviour Sanitizer. [Project-OSRM#6290](Project-OSRM#6290) - CHANGED: Use Conan instead of Mason to install code dependencies. [Project-OSRM#6284](Project-OSRM#6284) - CHANGED: Migrate to C++17. Update sol2 to 3.3.0. [Project-OSRM#6279](Project-OSRM#6279) - CHANGED: Update macOS CI image to macos-11. [Project-OSRM#6286](Project-OSRM#6286) - CHANGED: Enable even more clang-tidy checks. [Project-OSRM#6273](Project-OSRM#6273) - CHANGED: Configure CMake to not build flatbuffers tests and samples. [Project-OSRM#6274](Project-OSRM#6274) - CHANGED: Enable more clang-tidy checks. [Project-OSRM#6270](Project-OSRM#6270) - CHANGED: Configure clang-tidy job on CI. [Project-OSRM#6261](Project-OSRM#6261) - CHANGED: Use Github Actions for building container images [Project-OSRM#6138](Project-OSRM#6138) - CHANGED: Upgrade Boost dependency to 1.70 [Project-OSRM#6113](Project-OSRM#6113) - CHANGED: Upgrade Ubuntu CI builds to 20.04 [Project-OSRM#6119](Project-OSRM#6119) - CHANGED: Make building osrm-routed optional [Project-OSRM#6144](Project-OSRM#6144) - FIXED: Run all unit tests in CI [Project-OSRM#5248](Project-OSRM#5248) - FIXED: Fix installation of Mason CMake and 32 bit CI build [Project-OSRM#6170](Project-OSRM#6170) - FIXED: Fixed Node docs generation check in CI. [Project-OSRM#6058](Project-OSRM#6058) - CHANGED: Docker build, enabled arm64 build layer [Project-OSRM#6172](Project-OSRM#6172) - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [Project-OSRM#6175](Project-OSRM#6175) - FIXED: Bump CI complete meta job to ubuntu-20.04 [Project-OSRM#6323](Project-OSRM#6323) - CHANGED: Node packages are now scoped by @Project-OSRM [Project-OSRM#6386](Project-OSRM#6386) - Routing: - CHANGED: Lazily generate optional route path data [Project-OSRM#6045](Project-OSRM#6045) - FIXED: Completed support for no_entry and no_exit turn restrictions. [Project-OSRM#5988](Project-OSRM#5988) - ADDED: Add support for non-round-trips with a single fixed endpoint. [Project-OSRM#6050](Project-OSRM#6050) - FIXED: Improvements to maneuver override processing [Project-OSRM#6125](Project-OSRM#6125) - ADDED: Support snapping to multiple ways at an input location. [Project-OSRM#5953](Project-OSRM#5953) - FIXED: Fix snapping target locations to ways used in turn restrictions. [Project-OSRM#6339](Project-OSRM#6339) - ADDED: Support OSM traffic signal directions. [Project-OSRM#6153](Project-OSRM#6153) - FIXED: Ensure u-turn exists in intersection view. [Project-OSRM#6376](Project-OSRM#6376) - FIXED: Gracefully handle no-turn intersections in guidance processing. [Project-OSRM#6382](Project-OSRM#6382) - Profile: - CHANGED: Bicycle surface speeds [Project-OSRM#6212](Project-OSRM#6212) - Tools: - CHANGED: Do not generate intermediate .osrm file in osrm-extract. [Project-OSRM#6354](Project-OSRM#6354)

Due to some rather complex logic that tries to calculate intersection angles by looking further up the road, it's possible to return an intersection view that is missing a u-turn - something which is assumed to exist in later guidance calculations. We apply a fix here by ensuring the u-turn is always included in the returned view.

mjjbell commented Sep 25, 2022

View reviewed changes

mjjbell force-pushed the mbell/fix_intersection_uturn branch from fe1f789 to c22b0b7 Compare September 25, 2022 15:29

SiarheiFedartsou approved these changes Sep 25, 2022

View reviewed changes

SiarheiFedartsou reviewed Sep 25, 2022

View reviewed changes

mjjbell force-pushed the mbell/fix_intersection_uturn branch from c22b0b7 to dd1ace8 Compare September 26, 2022 22:25

mjjbell merged commit ef8f3d7 into Project-OSRM:master Sep 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure u-turn exists in intersection view #6376

Ensure u-turn exists in intersection view #6376

mjjbell commented Sep 25, 2022 •

edited

Loading

mjjbell Sep 25, 2022 •

edited

Loading

mjjbell Sep 25, 2022

SiarheiFedartsou Sep 25, 2022

Ensure u-turn exists in intersection view #6376

Ensure u-turn exists in intersection view #6376

Conversation

mjjbell commented Sep 25, 2022 • edited Loading

Context

Issue

Tasklist

Requirements / Relations

mjjbell Sep 25, 2022 • edited Loading

Choose a reason for hiding this comment

mjjbell Sep 25, 2022

Choose a reason for hiding this comment

SiarheiFedartsou Sep 25, 2022

Choose a reason for hiding this comment

mjjbell commented Sep 25, 2022 •

edited

Loading

mjjbell Sep 25, 2022 •

edited

Loading