-
Notifications
You must be signed in to change notification settings - Fork 406
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
fix(map-matching): Fix broken map matching for here traffic data #1430
Conversation
@aoles Is this PR ready? |
@MichaelsJP Yes, otherwise I wouldn't mark it as such. |
@aoles Are you sure proceeding without having at least basic unit tests for hmm module? |
Thanks @MichaelsJP! This is just a spin-off from #1403 meant to address some fundamental problems. Its main purpose is to restore some basic functionality around map patching in order to enable the implementation of API tests for routing with traffic. In fact, it is not even meant to be a complete solution to all of the problems, see e.g. #1434. Therefore, I believe any additional tests are beyond the scope of this PR. To summarize, I'm quite positive about merging the PR. Proper evaluation of the correctness of the HMM module will happen in #1435, and any potential unit tests should probably be added there. |
b8feaac
to
3f19bcd
Compare
3f19bcd
to
52370f8
Compare
Match to the ordering in `Coordinate` objects for proper distance calculation.
Circumvent issue of obtaining `Path` edges by calling the routing algorithm directly rather than creating a `GHRequest`.
Modifing the hash map inside of the loop seems to mess up the original loop iterator.
…object fields. There is no need of duplicating fields contained in the `Snap` instance which a `MatchPoint` holds.
52370f8
to
2903714
Compare
@aoles Please give a short heads-up on the status of this and what still needs to be done. Thanks ❤️ |
@MichaelsJP It's ready to be reviewed and merged since about two weeks now :) The API tests for #1403 were not meant to be part of this PR and will be implemented in a separate PR. |
Pull Request Checklist
have been resolved.
[Unreleased] heading.
along with a short description of what it is for, and documented this in the Pull Request (below).
(at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
importer etc.), I have generated longer distance routes for the affected profiles with different options
(avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end
points generated from the current live ORS.
If there are differences then the reasoning for these MUST be documented in the pull request.
and why the change was needed.
Information about the changes
Addresses the problem of traffic data not being matched to the graph which has been discovered when working on #1403.