search

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

Use hmm-lib 1.0.0 #66

Closed stefanholder closed 8 years ago

stefanholder commented 8 years ago

Copied hmm-lib release 1.0.0 and parts of offline_map_matching 1.0.0-SNAPSHOT.

Refactored MapMatching.java to use the new hmm-lib 1.0.0 API, which implements #61. Since QueryGraph.lookup() expects all candidates, the candidates of all time steps still need to be precomputed. However, it is no longer necessary to precompute paths, emission and transition probabilities.

Updated NOTICE.md with NOTICE content of bundled Apache 2.0 projects (hmm-lib and parts of offline_map_matching). Note that NOTICE.md should rather just be named NOTICE, see http://www.apache.org/dev/licensing-howto.html.

Needed to revert 450ddcd52efd76715dbea22df7d720bf4a889192 because of compilation errors.

karussell commented 8 years ago

Thanks a bunch - this is really nice!

A recent but simple refactoring in GH core/master is currently causing build failures at travis but I should be able to fix this (fast).

Note that NOTICE.md should rather just be named NOTICE

no problem, we can easily rename this (and others) if required (now or at any time later)

stefanholder commented 8 years ago

Removed revert of 450ddcd52efd76715dbea22df7d720bf4a889192 and travis is now building. However, I get compilation errors on my machine.

karussell commented 8 years ago

However, I get compilation errors on my machine.

Please fetch&rebuild the latest master of GH core.

stefanholder commented 8 years ago

Please fetch&rebuild the latest master of GH core.

Thanks, it's now working.

Note that NOTICE.md should rather just be named NOTICE

no problem, we can easily rename this (and others) if required (now or at any time later)

I just renamed NOTICE.md to NOTICE.

stefanholder commented 8 years ago

I created separate commits for copying hmm-lib 1.0.0 and for updating the offline_map_matching utility classes. This should also make the review easier.

karussell commented 8 years ago

(BTW: Tried the new github review thing ...)

Since QueryGraph.lookup() expects all candidates, the candidates of all time steps still need to be precomputed

You can also fill it with just the locations of two GPXEntries but then we need to pick the correct queryGraph for the routing algorithm

stefanholder commented 8 years ago

You can also fill it with just the locations of two GPXEntries but then we need to pick the correct queryGraph for the routing algorithm

Thanks for the hint, I can try to do it this way. Or do you think it's better to lookup all candidates at once, e.g. for performance reasons?

karussell commented 8 years ago

Thanks for the hint, I can try to do it this way. Or do you think it's better to lookup all candidates at once, e.g. for performance reasons?

Should be same performance I hope. We really need #68 :) ... but for this PR please keep it.

(BTW: Tried the new github review thing ...)

Nice now I get all messages through duplicate emails :(

I would like to provide the hmm-lib 1.0.0 as is. Changing the groupId afterwards (preferably after this pull request) is up to you.

Ok, sure.

So I can merge this now, right?

stefanholder commented 8 years ago

So I can merge this now, right?

Right.