Trying to fix issue 13 - Githubissues

graphhopper / map-matching

The map matching functionality is now located in the main repository https://github.com/graphhopper/graphhopper#map-matching

https://www.graphhopper.com/open-source/

Apache License 2.0

785 stars 274 forks source link

Trying to fix issue 13 #75

Closed karussell closed 7 years ago

karussell commented 7 years ago

Now here I stumbled over a problem that the time difference is 0 which leads to an infinite transitionLogProbability. Probably we have to handle those cases separately?

if (timeDiff == 0) {
    return 0;
}
Double transitionMetric = normalizedTransitionMetric(routeLength, linearDistance, timeDiff);
return Distributions.logExponentialDistribution(beta, transitionMetric);

Related #13

karussell commented 7 years ago

See the proposed simple fix. But maybe we should throw an error at this point and avoid passing 0 to this method?

michaz commented 7 years ago

These are logarithms of probabilities. Returning 0 means "for sure". Returning minus infinity is less wrong than returning 0, because the probability of being at two different places with time difference 0 is 0.

kodonnell commented 7 years ago

the probability of being at two different places with time difference 0 is 0

so we should check if they are actually at the same place too. If they are - very high transition probability. If they aren't, then low.

Alternatively:

throw an error. This is what I do in my case, as one should not get such data - and I don't want to be unknowingly processing erroneous data, etc.
filter out such entries - which probably makes sense to go here. We can either add this as (configurable) functionality, or allow the user to override the method if they want custom filtering (which gives us/them much more flexibility).

michaz commented 7 years ago

Maybe filter them out if they are the same location, and otherwise handle them like any other broken chain, possibly with a helpful comment: "you have identical timestamps on different points" added by the API/web layer? But to be continuous with the normal case, the library function should maybe just report it as a broken chain.

Or.. yes, we could also just require timestamps to be strictly increasing (by the API/web layer) and that's all.

karussell commented 7 years ago

@michaz would you mind to open a new issue for this? Maybe we should be strict in the core and for the web stuff we just attach artificial timestamps with a speed of ~20km/h (at least per default)?