Zero Distance Match Leg Causes Nan Duration #2229
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinkreiser
commented
Feb 10, 2020
| R"({"costing":"auto","format":"osrm","shape_match":"map_snap","shape":[ | ||
| {"lat": 52.0981280, "lon": 5.1297250, "type": "break", "time":10}, | ||
| {"lat": 52.0981280, "lon": 5.1297250, "type": "break", "time":169}], | ||
| "trace_options": {"interpolation_distance": 0}})", |
There was a problem hiding this comment.
this is important, we never saw much of this before because people generally dont want to skip interpolation. i would argue they still dont 😄
kevinkreiser
commented
Feb 10, 2020
| double dist_from_begin = (trivial_group ? end_pct : 1) - begin_pct; | ||
| double dist_to_end = end_pct - (trivial_group ? begin_pct : 0); | ||
| double begin_edge_scale = dist_from_begin > 0 ? 1 / dist_from_begin : 0; | ||
| double end_edge_scale = dist_to_end > 0 ? 1 / dist_to_end : 0; |
There was a problem hiding this comment.
zero-out the multiplier if its a degenerate leg
danpaz
approved these changes
Feb 10, 2020
|
docker build failed because ppa is randomly not reachable. its ephemeral. merging anyway |
yuzheyan
added a commit
that referenced
this pull request
Feb 11, 2020
* master: missed leg weight to 3 decimal places (#2227) Zero Distance Match Leg Causes Nan Duration (#2229) Fixed the sharp turn phrase (#2226) Cleanup in test Cleanup whitespace in directions.proto Cleanup unused argument etp in narrativebuilder Code review feedback: test vector references, style nit Update to front and back. j/k: move TrimBuild to other geometry utilities. Test coverage, export TrimShape into header namespace; check for empty shape. Strip const from primitive types. Fix pointer invalidation bug in TrimShape. prevent calling path_map_match when path_edges are empty. (#2219)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So... we allow degenerate routes, where the input is the same location twice and the output is a route with 0 length. The problem is, when we got smart about computing the duration we didnt figure for this. What happens is we get a divide by 0 which sends double to
infand then using that later on makes fornan. This PR adds a test, which fails with the old code, then it adds code to make sure that the denominator is positive. If the denominator is negative we set everything to 0.