Skip to content

Fix manuever overrides finding bug#6739

Merged
mjjbell merged 4 commits intoProject-OSRM:masterfrom
rezashokry:fix-maneuver-overrides-bug
Mar 24, 2024
Merged

Fix manuever overrides finding bug#6739
mjjbell merged 4 commits intoProject-OSRM:masterfrom
rezashokry:fix-maneuver-overrides-bug

Conversation

@rezashokry
Copy link
Copy Markdown
Contributor

Issue

I was trying to add some maneuver override relation to the map and I noticed some of them are not working.
I tried to find out why and I figured out this is because there is a vector of maneuver overrides which is assumed to be sorted to use binary search on it in lookup but its not sorted at the binary search stage.
The vector is already sorted in extract function but after the renumber in partition function the vector becomes unsorted.

Extraction:
https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/edge_based_graph_factory.cpp#L1225

Renumbering in partitioner: (which doesn't maintain the sorted array)
https://github.com/Project-OSRM/osrm-backend/blob/master/src/partitioner/partitioner.cpp#L163C1-L163C1

Finding maneuver overrides for each node:
https://github.com/Project-OSRM/osrm-backend/blob/master/src/engine/guidance/post_processing.cpp#L582

Binary search on the vector:
https://github.com/Project-OSRM/osrm-backend/blob/master/include/engine/datafacade/contiguous_internalmem_datafacade.hpp#L600

I just simply sort the vector after the rename function before the partition function writes it down.

Tasklist

Requirements / Relations

This pull request fixes this issue.

@mjjbell
Copy link
Copy Markdown
Member

mjjbell commented Mar 17, 2024

Good spot. Looks like it happened in #4907
I'll create a test case from your original example to confirm the fix.

@mjjbell
Copy link
Copy Markdown
Member

mjjbell commented Mar 17, 2024

As this is a bug related to the multi-level partitioning, it's difficult to add a neat Cucumber test case.

However, I've verified the fix using the example PBFs from the issue details - thanks for providing them, it's very helpful.

@mjjbell
Copy link
Copy Markdown
Member

mjjbell commented Mar 17, 2024

@rezashokry can you add an entry to CHANGELOG.md ?

@mjjbell mjjbell force-pushed the fix-maneuver-overrides-bug branch from 11adea8 to e1aecf2 Compare March 24, 2024 20:30
@mjjbell mjjbell merged commit 367933f into Project-OSRM:master Mar 24, 2024
eliseier pushed a commit to wanderlog/osrm-backend that referenced this pull request Mar 25, 2025
* sort manuever overrides vector after partition

---------

Co-authored-by: rshokri <[email protected]>
Co-authored-by: Michael Bell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi maneuove relation bug!

2 participants