diff --git a/map-data/issue-70.osm.gz b/map-data/issue-70.osm.gz new file mode 100644 index 00000000..88b1a3d5 Binary files /dev/null and b/map-data/issue-70.osm.gz differ diff --git a/matching-core/src/main/java/com/graphhopper/matching/DirectedVirtualEdgeQuadruple.java b/matching-core/src/main/java/com/graphhopper/matching/DirectedVirtualEdgeQuadruple.java new file mode 100644 index 00000000..29d36820 --- /dev/null +++ b/matching-core/src/main/java/com/graphhopper/matching/DirectedVirtualEdgeQuadruple.java @@ -0,0 +1,51 @@ +package com.graphhopper.matching; + +import java.util.ArrayList; +import com.graphhopper.routing.QueryGraph; +import com.graphhopper.routing.VirtualEdgeIteratorState; +import com.graphhopper.util.EdgeIterator; + +/* + * At a virtual node we get four virtual edges: + * + * GPX + * | + * | + * v1 | v2 + * --->--- --->---- + * N + * ---<--- ---<---- + * v3 v4 + * + * This is to represent this quadruple. We call it 'directed' in the sense that we allow only + * only of the edges to be used (by unfavoring the other three). + */ +public class DirectedVirtualEdgeQuadruple { + + public ArrayList quad = new ArrayList(); + public int favoured; + + public DirectedVirtualEdgeQuadruple(int virtualNode, QueryGraph queryGraph, int favoured) { + this.favoured = favoured; + EdgeIterator iter = queryGraph.createEdgeExplorer().setBaseNode(virtualNode); + while (iter.next()) { + quad.add((VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(iter.getEdge(), iter.getAdjNode())); + } + assert quad.size() == 2; + // add reverse + for (int i = 0; i < 2; i++) { + VirtualEdgeIteratorState e = quad.get(i); + // TODO: is this the correct way to reverse? Or the following? + // quad.add((VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(e.getEdge(), e.getBaseNode())); + quad.add((VirtualEdgeIteratorState) queryGraph.getEdgeIteratorState(e.getEdge(), e.getAdjNode())); + } + } + + public void setFavouringOfUnfavored(boolean favor) { + for (int i = 0; i < 4; i++) { + if (i != favoured) { + quad.get(i).setUnfavored(favor); + } + } + } +} diff --git a/matching-core/src/main/java/com/graphhopper/matching/GPXExtension.java b/matching-core/src/main/java/com/graphhopper/matching/GPXExtension.java index 1c357c8e..d9d89354 100644 --- a/matching-core/src/main/java/com/graphhopper/matching/GPXExtension.java +++ b/matching-core/src/main/java/com/graphhopper/matching/GPXExtension.java @@ -17,6 +17,9 @@ */ package com.graphhopper.matching; +import java.util.List; + +import com.graphhopper.routing.VirtualEdgeIteratorState; import com.graphhopper.storage.index.QueryResult; import com.graphhopper.util.GPXEntry; @@ -27,19 +30,28 @@ public class GPXExtension { final GPXEntry entry; final QueryResult queryResult; - final int gpxListIndex; + private boolean directed; + public DirectedVirtualEdgeQuadruple virtualEdgeQuadruple; - public GPXExtension(GPXEntry entry, QueryResult queryResult, int gpxListIndex) { + public GPXExtension(GPXEntry entry, QueryResult queryResult) { this.entry = entry; this.queryResult = queryResult; - this.gpxListIndex = gpxListIndex; + this.directed = false; + } + + public GPXExtension(GPXEntry entry, QueryResult queryResult, DirectedVirtualEdgeQuadruple virtualEdgeQuadruple) { + this(entry, queryResult); + this.virtualEdgeQuadruple = virtualEdgeQuadruple; + this.directed = true; } + public boolean isDirected() { + return directed; + } + @Override public String toString() { - return "entry:" + entry - + ", query distance:" + queryResult.getQueryDistance() - + ", gpxListIndex:" + gpxListIndex; + return "entry:" + entry + ", query distance:" + queryResult.getQueryDistance(); } public QueryResult getQueryResult() { diff --git a/matching-core/src/main/java/com/graphhopper/matching/MapMatching.java b/matching-core/src/main/java/com/graphhopper/matching/MapMatching.java index 08387cbc..06dd78cf 100644 --- a/matching-core/src/main/java/com/graphhopper/matching/MapMatching.java +++ b/matching-core/src/main/java/com/graphhopper/matching/MapMatching.java @@ -17,6 +17,7 @@ */ package com.graphhopper.matching; +import com.graphhopper.routing.VirtualEdgeIteratorState; import com.graphhopper.GraphHopper; import com.graphhopper.matching.util.HmmProbabilities; import com.graphhopper.matching.util.TimeStep; @@ -39,6 +40,7 @@ import com.graphhopper.util.*; import com.graphhopper.util.shapes.GHPoint; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -161,36 +163,69 @@ public MatchResult doWork(List gpxList) { + gpxList.size() + "). Correct format?"); } - final EdgeFilter edgeFilter = new DefaultEdgeFilter(algoOptions.getWeighting().getFlagEncoder()); - - // Compute all candidates first. - // TODO: Generate candidates on-the-fly within computeViterbiSequence() if this does not - // degrade performance. - final List allCandidates = new ArrayList<>(); - List> timeSteps = createTimeSteps(gpxList, - edgeFilter, allCandidates); - - if (allCandidates.size() < 2) { - throw new IllegalArgumentException("Too few matching coordinates (" - + allCandidates.size() + "). Wrong region imported?"); + // filter the entries: + List filteredGPXEntries = filterGPXEntries(gpxList); + if (filteredGPXEntries.size() < 2) { + throw new IllegalStateException("Only " + filteredGPXEntries.size() + " filtered GPX entries (from " + gpxList.size() + "), but two or more are needed"); } - if (timeSteps.size() < 2) { - throw new IllegalStateException("Coordinates produced too few time steps " - + timeSteps.size() + ", gpxList:" + gpxList.size()); - } - + + // now find each of the entries in the graph: + final EdgeFilter edgeFilter = new DefaultEdgeFilter(algoOptions.getWeighting().getFlagEncoder()); + List> queriesPerEntry = findGPXEntriesInGraph(filteredGPXEntries, edgeFilter); + + // now look up the entries up in the graph: final QueryGraph queryGraph = new QueryGraph(routingGraph).setUseEdgeExplorerCache(true); - queryGraph.lookup(allCandidates); + List allQueryResults = new ArrayList(); + for (List qrs: queriesPerEntry) + allQueryResults.addAll(qrs); + queryGraph.lookup(allQueryResults); - List> seq = computeViterbiSequence(timeSteps, - gpxList, queryGraph); + // create candidates from the entries in the graph (a candidate is basically an entry + direction): + List> timeSteps = createTimeSteps(filteredGPXEntries, queriesPerEntry, queryGraph); + // viterbify: + List> seq = computeViterbiSequence(timeSteps, gpxList, queryGraph); + + // finally, extract the result: final EdgeExplorer explorer = queryGraph.createEdgeExplorer(edgeFilter); - MatchResult matchResult = computeMatchResult(seq, gpxList, allCandidates, explorer); + MatchResult matchResult = computeMatchResult(seq, gpxList, queriesPerEntry, explorer); +// MatchResult matchResult = computeMatchResult(seq, gpxList, queriesPerEntry, explorer); return matchResult; } - + + /** + * Filters GPX entries to only those which will be used for map matching (i.e. those which + * are separated by at least 2 * measurementErrorSigman + */ + private List filterGPXEntries(List gpxList) { + List filtered = new ArrayList(); + GPXEntry prevEntry = null; + int last = gpxList.size() - 1; + for (int i = 0; i <= last; i++) { + GPXEntry gpxEntry = gpxList.get(i); + if (i == 0 || i == last || distanceCalc.calcDist( + prevEntry.getLat(), prevEntry.getLon(), + gpxEntry.getLat(), gpxEntry.getLon()) > 2 * measurementErrorSigma) { + filtered.add(gpxEntry); + prevEntry = gpxEntry; + } + } + return filtered; + } + /** + * Find the possible locations of each qpxEntry in the graph. + */ + private List> findGPXEntriesInGraph(List gpxList, EdgeFilter edgeFilter) { + + List> gpxEntryLocations = new ArrayList>(); + for (GPXEntry gpxEntry : gpxList) { + gpxEntryLocations.add(locationIndex.findNClosest(gpxEntry.lat, gpxEntry.lon, edgeFilter, measurementErrorSigma)); + } + return gpxEntryLocations; + } + + /** * Creates TimeSteps for the GPX entries but does not create emission or * transition probabilities. @@ -198,33 +233,48 @@ public MatchResult doWork(List gpxList) { * @param outAllCandidates output parameter for all candidates, must be an * empty list. */ - private List> createTimeSteps(List gpxList, - EdgeFilter edgeFilter, List outAllCandidates) { - int indexGPX = 0; - TimeStep prevTimeStep = null; + private List> createTimeSteps(List filteredGPXEntries, + List> queriesPerEntry, QueryGraph queryGraph) { + final List> timeSteps = new ArrayList<>(); - for (GPXEntry gpxEntry : gpxList) { - if (prevTimeStep == null - || distanceCalc.calcDist( - prevTimeStep.observation.getLat(), prevTimeStep.observation.getLon(), - gpxEntry.getLat(), gpxEntry.getLon()) > 2 * measurementErrorSigma - // always include last point - || indexGPX == gpxList.size() - 1) { - final List queryResults = locationIndex.findNClosest( - gpxEntry.lat, gpxEntry.lon, - edgeFilter, measurementErrorSigma); - outAllCandidates.addAll(queryResults); - final List candidates = new ArrayList<>(); - for (QueryResult candidate : queryResults) { - candidates.add(new GPXExtension(gpxEntry, candidate, indexGPX)); - } - final TimeStep timeStep - = new TimeStep<>(gpxEntry, candidates); - timeSteps.add(timeStep); - prevTimeStep = timeStep; + int n = filteredGPXEntries.size(); + assert queriesPerEntry.size() == n; + for (int i = 0; i < n; i++) { + + GPXEntry gpxEntry = filteredGPXEntries.get(i); + List queryResults = queriesPerEntry.get(i); + + List candidates = new ArrayList(); + for (QueryResult qr: queryResults) { + int closestNode = qr.getClosestNode(); + if (queryGraph.isVirtualNode(closestNode)) { + // if virtual, create four candidates: one for each virtual edge around the virtual node ... + List virtualEdgeQuads = new ArrayList(); + for (int favoured = 0; favoured < 4; favoured++) { + DirectedVirtualEdgeQuadruple veq = new DirectedVirtualEdgeQuadruple(closestNode, queryGraph, favoured); + virtualEdgeQuads.add(veq); + // create candidate + QueryResult vqr = new QueryResult(qr.getQueryPoint().lat, qr.getQueryPoint().lon); + vqr.setQueryDistance(qr.getQueryDistance()); + vqr.setClosestNode(qr.getClosestNode()); + vqr.setWayIndex(qr.getWayIndex()); + vqr.setSnappedPosition(qr.getSnappedPosition()); + vqr.setClosestEdge(qr.getClosestEdge()); + vqr.calcSnappedPoint(distanceCalc); + GPXExtension candidate = new GPXExtension(gpxEntry, vqr, veq); + candidates.add(candidate); + } + + } else { + // just add the real edge, undirected + GPXExtension candidate = new GPXExtension(gpxEntry, qr); + candidates.add(candidate); + } } - indexGPX++; + + final TimeStep timeStep = new TimeStep<>(gpxEntry, candidates); + timeSteps.add(timeStep); } return timeSteps; } @@ -301,9 +351,17 @@ private void computeTransitionProbabilities(TimeStep> seq, - List gpxList, List allCandidates, + List gpxList, List> queriesPerEntry, EdgeExplorer explorer) { // every virtual edge maps to its real edge where the orientation is already correct! // TODO use traversal key instead of string! final Map virtualEdgesMap = new HashMap<>(); - for (QueryResult candidate : allCandidates) { - fillVirtualEdges(virtualEdgesMap, explorer, candidate); + for (List queryResults: queriesPerEntry) { + for (QueryResult qr: queryResults) { + fillVirtualEdges(virtualEdgesMap, explorer, qr); + } } MatchResult matchResult = computeMatchedEdges(seq, virtualEdgesMap); diff --git a/matching-core/src/test/java/com/graphhopper/matching/MapMatching2Test.java b/matching-core/src/test/java/com/graphhopper/matching/MapMatching2Test.java index bb093459..f2c83630 100644 --- a/matching-core/src/test/java/com/graphhopper/matching/MapMatching2Test.java +++ b/matching-core/src/test/java/com/graphhopper/matching/MapMatching2Test.java @@ -62,4 +62,26 @@ public void testIssue13() { assertEquals(mr.getGpxEntriesLength(), mr.getMatchLength(), 2.5); assertEquals(28790, mr.getMatchMillis(), 50); } + + @Test + public void testIssue70() { + CarFlagEncoder encoder = new CarFlagEncoder(); + TestGraphHopper hopper = new TestGraphHopper(); + hopper.setDataReaderFile("../map-data/issue-70.osm.gz"); + hopper.setGraphHopperLocation("../target/mapmatchingtest-70"); + hopper.setEncodingManager(new EncodingManager(encoder)); + hopper.importOrLoad(); + + AlgorithmOptions opts = AlgorithmOptions.start().build(); + MapMatching mapMatching = new MapMatching(hopper, opts); + + List inputGPXEntries = new GPXFile(). + doImport("./src/test/resources/issue-70.gpx").getEntries(); + MatchResult mr = mapMatching.doWork(inputGPXEntries); + + assertEquals(Arrays.asList("Милана Видака", "Милана Видака", "Милана Видака", + "Бранка Радичевића", "Бранка Радичевића", "Здравка Челара"), + fetchStreets(mr.getEdgeMatches())); + // TODO: length/time + } } diff --git a/matching-core/src/test/java/com/graphhopper/matching/MapMatchingTest.java b/matching-core/src/test/java/com/graphhopper/matching/MapMatchingTest.java index df5c6c5f..3f2ed3fe 100644 --- a/matching-core/src/test/java/com/graphhopper/matching/MapMatchingTest.java +++ b/matching-core/src/test/java/com/graphhopper/matching/MapMatchingTest.java @@ -82,7 +82,7 @@ public static Collection algoOptions() { // force CH AlgorithmOptions chOpts = AlgorithmOptions.start() - .maxVisitedNodes(40) + .maxVisitedNodes(1000) .hints(new PMap().put(Parameters.CH.DISABLE, false)) .build(); @@ -208,7 +208,8 @@ public void testSmallSeparatedSearchDistance() { MatchResult mr = mapMatching.doWork(inputGPXEntries); assertEquals(Arrays.asList("Weinligstraße", "Weinligstraße", "Weinligstraße", "Fechnerstraße", "Fechnerstraße"), fetchStreets(mr.getEdgeMatches())); - assertEquals(mr.getGpxEntriesLength(), mr.getMatchLength(), 11); + // TODO: test these individually, not combined? I.e. this would pass if both were zero ... + assertEquals(mr.getGpxEntriesLength(), mr.getMatchLength(), 11); // TODO: this should be around 300m according to Google ... need to check assertEquals(mr.getGpxEntriesMillis(), mr.getMatchMillis(), 3000); } diff --git a/matching-core/src/test/resources/issue-70.gpx b/matching-core/src/test/resources/issue-70.gpx new file mode 100644 index 00000000..ce8c4faa --- /dev/null +++ b/matching-core/src/test/resources/issue-70.gpx @@ -0,0 +1,25 @@ + + + + converted track + + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + 382 + + + \ No newline at end of file diff --git a/matching-web/src/main/java/com/graphhopper/matching/http/MatchServlet.java b/matching-web/src/main/java/com/graphhopper/matching/http/MatchServlet.java index b43d688b..9b584449 100644 --- a/matching-web/src/main/java/com/graphhopper/matching/http/MatchServlet.java +++ b/matching-web/src/main/java/com/graphhopper/matching/http/MatchServlet.java @@ -64,6 +64,7 @@ public class MatchServlet extends GraphHopperServlet { public void doPost(HttpServletRequest httpReq, HttpServletResponse httpRes) throws ServletException, IOException { + logger.info("posted"); String infoStr = httpReq.getRemoteAddr() + " " + httpReq.getLocale(); String inType = "gpx"; String contentType = httpReq.getContentType(); diff --git a/matching-web/src/test/java/com/graphhopper/matching/http/MatchResultToJsonTest.java b/matching-web/src/test/java/com/graphhopper/matching/http/MatchResultToJsonTest.java index 2adda29a..2d993e9a 100644 --- a/matching-web/src/test/java/com/graphhopper/matching/http/MatchResultToJsonTest.java +++ b/matching-web/src/test/java/com/graphhopper/matching/http/MatchResultToJsonTest.java @@ -58,8 +58,8 @@ public GHPoint3D getSnappedPoint() { } }; - list.add(new GPXExtension(new GPXEntry(-3.4446, -38.9996, 100000), queryResult1, 1)); - list.add(new GPXExtension(new GPXEntry(-3.4448, -38.9999, 100001), queryResult2, 1)); + list.add(new GPXExtension(new GPXEntry(-3.4446, -38.9996, 100000), queryResult1)); + list.add(new GPXExtension(new GPXEntry(-3.4448, -38.9999, 100001), queryResult2)); return list; }