Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to maneuver override processing #6215

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

mjjbell
Copy link
Member

@mjjbell mjjbell commented Mar 5, 2022

Issue

This change unblocks the osrm-extract debug build, which is currently failing on a maneuver override assertion.

The processing of maneuver overrides currently has three issues:

  • It assumes the via node(s) can't be compressed (the failing assertion)
  • It can't handle via-paths containing incompressible nodes
  • It doesn't interop with turn restrictions on the same path

Turn restrictions and maneuver overrides both use the same from-via-to path representation.
Therefore, we can fix these issues by consolidating their structures and reusing the path representation for turn restrictions, which already is robust to the above issues.

This also simplifies some of the codebase by removing maneuver override specific path processing.

There are ~100 maneuver overrides in the OSM database, so the impact on processing and routing will be minimal.
But who knows, with improved support, it might encourage further adoption.

Tasklist

Requirements / Relations

Fixes #6126

@mjjbell mjjbell force-pushed the mbell/maneouvre_assert branch 2 times, most recently from e226cb9 to 05bc3f3 Compare March 5, 2022 16:14
@mjjbell mjjbell force-pushed the mbell/maneouvre_assert branch 3 times, most recently from 061d59b to a71b651 Compare August 23, 2022 09:07
@mjjbell mjjbell mentioned this pull request Aug 23, 2022
7 tasks
Copy link
Member

@SiarheiFedartsou SiarheiFedartsou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it is quite hard to review such PRs though - have no deep experience with OSRM codebase yet

This change unblocks the osrm-extract debug build, which is
currently failing on a maneuver override assertion.

The processing of maneuver overrides currently has three issues
- It assumes the via node(s) can't be compressed (the failing assertion)
- It can't handle via-paths containing incompressible nodes
- It doesn't interop with turn restriction on the same path

Turn restrictions and maneuver overrides both use the same
from-via-to path representation.
Therefore, we can fix these issues by consolidating their
structures and reusing the path representation for
turn restrictions, which already is robust to the above
issues.

This also simplifies some of the codebase by removing maneuver
override specific path processing.

There are ~100 maneuver overrides in the OSM database, so the
impact on processing and routing will be minimal.
@mjjbell mjjbell merged commit a98074a into Project-OSRM:master Aug 24, 2022
mattwigway pushed a commit to mattwigway/osrm-backend that referenced this pull request Jul 20, 2023
This change unblocks the osrm-extract debug build, which is
currently failing on a maneuver override assertion.

The processing of maneuver overrides currently has three issues
- It assumes the via node(s) can't be compressed (the failing assertion)
- It can't handle via-paths containing incompressible nodes
- It doesn't interop with turn restriction on the same path

Turn restrictions and maneuver overrides both use the same
from-via-to path representation.
Therefore, we can fix these issues by consolidating their
structures and reusing the path representation for
turn restrictions, which already is robust to the above
issues.

This also simplifies some of the codebase by removing maneuver
override specific path processing.

There are ~100 maneuver overrides in the OSM database, so the
impact on processing and routing will be minimal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

osrm-extract assert triggering on manoeuvre override relation
2 participants