Hi Steve, I am now sure that the 1st version of the patch is wrong. I think I understand now how removeShortArcsByMergingNodes() works: It measures the distance between two road junctions. If this distance is shorter than the given minimum length, it replaces the coord of the previous junction with the latter one (or vice verse, depending on boundary issues) and remembers that this coord was replaced. Any other road using the replaced coord is changed so that it also uses the replacement. Doing this some ways end up having only one point. These ways are removed. So, the algorithm has to iterate over all roads again when a coord was replaced. See eg. http://www.openstreetmap.org/browse/way/196384413 in the first pass, node http://www.openstreetmap.org/browse/node/2067273965 is replaced by http://www.openstreetmap.org/browse/node/2067273963 in the 2nd pass the way 196384413 is removed because it only has one point left. So, in short, I think the best solution until now is the one we have in r2448, and I think we should adapt the unit test. I still don't understand why a short arc breaks routing, I assume this is caused by some Garmin limit. Ciao, Gerd
Date: Thu, 17 Jan 2013 14:28:02 +0000 From: steve@parabola.me.uk To: mkgmap-dev@lists.mkgmap.org.uk Subject: [mkgmap-dev] Remove short arcs
Hi Gerd
Yes, today I tried to put it into RoadNetWork, but then I found out that this routine sees the roads after they were split into smaller parts by StyledConverter. So, a lot more program logic has to be changed, and I think
Yes, most of that code should be moved too. And anyway, shouldn't the short arc code be run after cutting the roads up anyway? It might also be possible to combine the two functions.
that I don't understand enough for that :-( I hope you can solve this, because I am not that happy with the removeShortArcs_v3.patch as well.
I don't understand it either. I suspect that it might be possible to solve the actual problem that removeShortArcs is attempting to solve, at the level of building the NOD file. But the only way to find out would be to find or construct cases that don't route properly as tests and experiment.
In the short term we need to decide on something to do. We could disable the failing tests for the moment, go back to the version that didn't iterate multiple times over the roads, or go back to running it over all lines.
Running over all lines is a step backwards in my opinion. The second option seems fine to me, as long as there are no error messages about short arcs. Even though the results are different they could well be for the better. The comments about contour lines in that routine make me think that it could be simplified now that we are not running it on contours. Lastly we could disable the failing tests for now, since they are false failures.
So it depends on what you think about the original patch that didn't run multiple times over the ways. If you think it is definitely broken then we should @Ignore the failing test, and I will try out ways of re-structuring all that code. I've wanted to do it for a long time since I think that it is a barrier to any routing improvements.
..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev