Hi Gerd I've re-enabled the nightly builds but the last one is 2447 as from 2448 there are test failures. One is because the roads are now added right at the end and the test does not call the converted end() method. The attached patch fixes that. The other is partially the same, but it appears that the resulting ways are also removed by the removeShortArcs routine. ..Steve
Hi Steve, Steve Ratcliffe wrote
I've re-enabled the nightly builds but the last one is 2447 as from 2448 there are test failures.
One is because the roads are now added right at the end and the test does not call the converted end() method. The attached patch fixes that.
Thanks. I should have found this myself, but the tests produce so many failure messages that I did not notice this additional one. Or maybe it is not normal to have 16 failures? This is my alltests-fails.html: alltests-fails.zip <http://gis.19327.n5.nabble.com/file/n5744571/alltests-fails.zip> Steve Ratcliffe wrote
The other is partially the same, but it appears that the resulting ways are also removed by the removeShortArcs routine.
Hmm, that one is difficult because the patched test does something completely different compared to the ElementSaver: ElementSaver calls converter.convertWay() for all ways and then finally converter.end() The patched StyleTester.runSimpleTest() calls xyz.convertWay() directly followed by xyz.end() for each way. The failing test could be changed to use non-road lines, but I think this not what we want. The javadoc in interface OSMConverter says that convertWay() should do the conversion, but with r2448 a part of the conversion is done in end() , so the whole program flow was changed. Should I try to change the tests so that they work like ElementSaver ? That would mean we can only test the result after calling the final end() Or do you see a better place for the removeShortArcs() routine? I thought about putting it into MapBuilder, but at this place we would not be able to report warning with the Way.toBrowseURL() method. Ciao, Gerd -- View this message in context: http://gis.19327.n5.nabble.com/Test-failure-in-r2448-tp5744543p5744571.html Sent from the Mkgmap Development mailing list archive at Nabble.com.
Hi Gerd
Thanks. I should have found this myself, but the tests produce so many failure messages that I did not notice this additional one. Or maybe it is not normal to have 16 failures?
No it is not normal - on unix anyway ;) I'm suprised that no one has complained before, given that most people run on windows. Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now. ..Steve
I think it is normal to have failures building mkgmap - been this way forever on Windows (I always do ant clean before compiling, without doing so, errors are very rare).... here is how it looks for me on windows (empty lines deleted for better readability in email): /d:\Garmin\mkgmap_svn_trunk>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_svn_trunk\build\classes ivy-availability: download-ivy: init-ivy: [ivy:configure] :: Ivy 2.2.0 - 20100923230623 :: http://ant.apache.org/ivy/ :: [ivy:configure] :: loading settings :: file = d:\Garmin\mkgmap_svn_trunk\ivysettings.xml resolve-compile: compile: [javac] Compiling 463 source files to d:\Garmin\mkgmap_svn_trunk\build\classes [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:454: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:489: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. [javac] 2 warnings build: [copy] Copying 438 files to d:\Garmin\mkgmap_svn_trunk\build\classes svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_svn_trunk\build\classes\mkgmap-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_svn_trunk\dist\mkgmap.jar BUILD SUCCESSFUL Total time: 27 seconds/ And here for splitter on Windows: /D:\Garmin>cd d:\garmin\mkgmap_splitter_svn d:\Garmin\mkgmap_splitter_svn>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-output compile: [javac] Compiling 68 source files to d:\Garmin\mkgmap_splitter_svn\build\classes [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. compile.tests: [javac] Compiling 5 source files to d:\Garmin\mkgmap_splitter_svn\build\test-classes run.tests: [testng] [Parser] Running: [testng] Ant suite [testng] [testng] Wrote 999 test pairs to C:\temp\test8256805846442018905.tmp [testng] temporary file C:\temp\test8256805846442018905.tmp was deleted [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] [testng] =============================================== [testng] Ant suite [testng] Total tests run: 11, Failures: 0, Skips: 0 [testng] =============================================== [testng] build: svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_splitter_svn\build\classes\splitter-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_splitter_svn\dist\splitter.jar [copy] Copying 1 file to d:\Garmin\mkgmap_splitter_svn\dist\src BUILD SUCCESSFUL Total time: 7 seconds/ On 15.01.2013 12:31, Steve Ratcliffe wrote:
Hi Gerd
Thanks. I should have found this myself, but the tests produce so many failure messages that I did not notice this additional one. Or maybe it is not normal to have 16 failures? No it is not normal - on unix anyway ;) I'm suprised that no one has complained before, given that most people run on windows.
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org
Hi Felix, to run the mkgmap unit tests, please execute ant obtain-test-input-files followed by ant test Besides that I see similar messages when building with jdk 1.7. Gerd Felix Hartmann-2 wrote
I think it is normal to have failures building mkgmap - been this way forever on Windows (I always do ant clean before compiling, without doing so, errors are very rare)....
here is how it looks for me on windows (empty lines deleted for better readability in email): /d:\Garmin\mkgmap_svn_trunk>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_svn_trunk\build\classes ivy-availability: download-ivy: init-ivy: [ivy:configure] :: Ivy 2.2.0 - 20100923230623 :: http://ant.apache.org/ivy/ :: [ivy:configure] :: loading settings :: file = d:\Garmin\mkgmap_svn_trunk\ivysettings.xml resolve-compile: compile: [javac] Compiling 463 source files to d:\Garmin\mkgmap_svn_trunk\build\classes [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:454: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:489: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. [javac] 2 warnings build: [copy] Copying 438 files to d:\Garmin\mkgmap_svn_trunk\build\classes svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_svn_trunk\build\classes\mkgmap-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_svn_trunk\dist\mkgmap.jar BUILD SUCCESSFUL Total time: 27 seconds/
And here for splitter on Windows:
/D:\Garmin>cd d:\garmin\mkgmap_splitter_svn d:\Garmin\mkgmap_splitter_svn>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-output compile: [javac] Compiling 68 source files to d:\Garmin\mkgmap_splitter_svn\build\classes [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. compile.tests: [javac] Compiling 5 source files to d:\Garmin\mkgmap_splitter_svn\build\test-classes run.tests: [testng] [Parser] Running: [testng] Ant suite [testng] [testng] Wrote 999 test pairs to C:\temp\test8256805846442018905.tmp [testng] temporary file C:\temp\test8256805846442018905.tmp was deleted [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] [testng] =============================================== [testng] Ant suite [testng] Total tests run: 11, Failures: 0, Skips: 0 [testng] =============================================== [testng] build: svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_splitter_svn\build\classes\splitter-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_splitter_svn\dist\splitter.jar [copy] Copying 1 file to d:\Garmin\mkgmap_splitter_svn\dist\src BUILD SUCCESSFUL Total time: 7 seconds/
On 15.01.2013 12:31, Steve Ratcliffe wrote:
Hi Gerd
Thanks. I should have found this myself, but the tests produce so many failure messages that I did not notice this additional one. Or maybe it is not normal to have 16 failures? No it is not normal - on unix anyway ;) I'm suprised that no one has complained before, given that most people run on windows.
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
..Steve _______________________________________________ mkgmap-dev mailing list
mkgmap-dev@.org
-- keep on biking and discovering new trails
Felix openmtbmap.org & www.velomap.org
_______________________________________________ mkgmap-dev mailing list
mkgmap-dev@.org
-- View this message in context: http://gis.19327.n5.nabble.com/Test-failure-in-r2448-tp5744543p5744630.html Sent from the Mkgmap Development mailing list archive at Nabble.com.
Well, I just thought I would post those errors, don't think they were posted before. To my knowledge they don't matter, but if Steve is working on fixing errors, then maybe he can fix them too, as newcomers might be bewildered by them. unit tests would probably fail for me too, but I don't even know what they are good for... On 15.01.2013 12:58, GerdP wrote:
Hi Felix,
to run the mkgmap unit tests, please execute ant obtain-test-input-files followed by ant test
Besides that I see similar messages when building with jdk 1.7.
Gerd
Felix Hartmann-2 wrote
I think it is normal to have failures building mkgmap - been this way forever on Windows (I always do ant clean before compiling, without doing so, errors are very rare)....
here is how it looks for me on windows (empty lines deleted for better readability in email): /d:\Garmin\mkgmap_svn_trunk>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_svn_trunk\build\classes ivy-availability: download-ivy: init-ivy: [ivy:configure] :: Ivy 2.2.0 - 20100923230623 :: http://ant.apache.org/ivy/ :: [ivy:configure] :: loading settings :: file = d:\Garmin\mkgmap_svn_trunk\ivysettings.xml resolve-compile: compile: [javac] Compiling 463 source files to d:\Garmin\mkgmap_svn_trunk\build\classes [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:454: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:489: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. [javac] 2 warnings build: [copy] Copying 438 files to d:\Garmin\mkgmap_svn_trunk\build\classes svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_svn_trunk\build\classes\mkgmap-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_svn_trunk\dist\mkgmap.jar BUILD SUCCESSFUL Total time: 27 seconds/
And here for splitter on Windows:
/D:\Garmin>cd d:\garmin\mkgmap_splitter_svn d:\Garmin\mkgmap_splitter_svn>start /b /wait ant dist Buildfile: build.xml prepare: [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-classes [mkdir] Created dir: d:\Garmin\mkgmap_splitter_svn\build\test-output compile: [javac] Compiling 68 source files to d:\Garmin\mkgmap_splitter_svn\build\classes [javac] Note: Some input files use unchecked or unsafe operations. [javac] Note: Recompile with -Xlint:unchecked for details. compile.tests: [javac] Compiling 5 source files to d:\Garmin\mkgmap_splitter_svn\build\test-classes run.tests: [testng] [Parser] Running: [testng] Ant suite [testng] [testng] Wrote 999 test pairs to C:\temp\test8256805846442018905.tmp [testng] temporary file C:\temp\test8256805846442018905.tmp was deleted [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] SparseLong2ShortMapInline: Allocating three-tier structure to save area info (HashMap->vector->chunkvector) [testng] [testng] =============================================== [testng] Ant suite [testng] Total tests run: 11, Failures: 0, Skips: 0 [testng] =============================================== [testng] build: svn-version: [exec] Result: 1 git-version: check-version: version-file: [propertyfile] Creating new property file: d:\Garmin\mkgmap_splitter_svn\build\classes\splitter-version.properties dist: [jar] Building jar: d:\Garmin\mkgmap_splitter_svn\dist\splitter.jar [copy] Copying 1 file to d:\Garmin\mkgmap_splitter_svn\dist\src BUILD SUCCESSFUL Total time: 7 seconds/
On 15.01.2013 12:31, Steve Ratcliffe wrote:
Hi Gerd
Thanks. I should have found this myself, but the tests produce so many failure messages that I did not notice this additional one. Or maybe it is not normal to have 16 failures? No it is not normal - on unix anyway ;) I'm suprised that no one has complained before, given that most people run on windows.
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
..Steve _______________________________________________ mkgmap-dev mailing list
mkgmap-dev@.org
http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- keep on biking and discovering new trails
Felix openmtbmap.org & www.velomap.org
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@.org http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
-- View this message in context: http://gis.19327.n5.nabble.com/Test-failure-in-r2448-tp5744543p5744630.html Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org
On 15/01/13 11:50, Felix Hartmann wrote:
/ [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:454: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\osmstyle\StyleImpl.java:489: warning: unreachable catch clause [javac] } catch (IOException e) { [javac] ^ [javac] thrown type FileNotFoundException has already been caught/
Thanks, although I knew that the code had that problem (its harmless as you suspected), I didn't know that some people were seeing a scary error message during compile. As you say that could put off people using it for the first time, so I shall fix that along with the tests. ..Steve
Hi Steve,
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
Thanks. I think on windows a file must not be open, so we probably have to code some close() calls. Eclipse has a function to show warnings for potentially missing close() calls. I fixed them for splitter, but I see 28 of them for mkgmap and I don't want to fix something that is not broken. Ciao, Gerd
On 15/01/2013 13:02, Gerd Petermann wrote:
Hi Steve,
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
Thanks. I think on windows a file must not be open, so we probably have to code some close() calls. Eclipse has a function to show warnings for potentially missing close() calls.
Almost all the real ones were in the test code, since I didn't want any try/finally clutter in there. The attached patch fixes. There is one test failure which I believe is a genuine bug on windows, which I'll look into some time later. I'll check this in when I've tried it out on unix again. ..Steve
Hi Steve, I had a few problems with the patch because the hunks for test/func/lib/TestUtils.java were rejected. Maybe because it was based on r2330 ? Anyway, it solves most of the failures on windwos with jkd 1.6 :-) I thought again about the removeShortArgs test issue. I did not yet find a simple solution. Probably the StyledConverter class is not the right place for the removeShortArcsByMergingNodes() method, I'll try to move it down in the chain to the MapDetails class. Please let me know when you prefer a different solution. Ciao, Gerd Date: Tue, 15 Jan 2013 15:45:26 +0000 From: steve@parabola.me.uk To: mkgmap-dev@lists.mkgmap.org.uk Subject: Re: [mkgmap-dev] Test failure in r2448 On 15/01/2013 13:02, Gerd Petermann wrote:
Hi Steve,
Its difficult to delete files on Windows compared to Unix. I'm part way through fixing it now.
Thanks. I think on windows a file must not be open, so we probably have to code some close() calls. Eclipse has a function to show warnings for potentially missing close() calls.
Almost all the real ones were in the test code, since I didn't want any try/finally clutter in there. The attached patch fixes. There is one test failure which I believe is a genuine bug on windows, which I'll look into some time later. I'll check this in when I've tried it out on unix again. ..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Gerd,
I had a few problems with the patch because the hunks for test/func/lib/TestUtils.java were rejected. Maybe because it was based on r2330 ?
Oh yes, sorry it was against whatever version was current last time I looked at it on Windows;) I also decided that the last remaining windows-only failure was a test bug, since the definition of an absolute file path is different on Windows, so I fixed that too.
I thought again about the removeShortArgs test issue. I did not yet find a simple solution. Probably the StyledConverter class is not the right place for the removeShortArcsByMergingNodes() method, I'll try to move it down in the chain to the MapDetails class. Please let me know when you prefer a different solution.
The functionality belongs in RoadNetwork or lower as it is not OSM specific. I have more thoughts and I'll continue on a more appropriate thread. ..Steve
Hi Steve, Steve Ratcliffe wrote
Probably the StyledConverter class is not the right place for the removeShortArcsByMergingNodes() method, I'll try to move it down in the chain to the MapDetails class. Please let me know when you prefer a different solution.
The functionality belongs in RoadNetwork or lower as it is not OSM specific. I have more thoughts and I'll continue on a more appropriate thread.
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 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. Gerd -- View this message in context: http://gis.19327.n5.nabble.com/Test-failure-in-r2448-tp5744543p5744936.html Sent from the Mkgmap Development mailing list archive at Nabble.com.
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
Hi Steve, I did not test the first version of the patch very deeply, but I think that we must remove code that nobody understands as fast as possible, esp. as correct routing is one of the most important features. I never understood why the original code iterates multiple times. So, it would be great if that is not needed and we can use the first version of my patch. If you don't mind to change the unit test for a while, let's do this: 1) you change the unit test so that r2448 and later are tested ok 2) I try to find out if the iteration is strictly needed or not. If it turns out to be needed we (you) can start re-writing the code. reg. test data: I did not yet try to understand why a short arc brakes routing. I assume it is because you can't calculate the bearing with an appropriate precision? I just know that I see error messages like "road ... contains zero length arc at ... " or "road ... contains consecutive identical nodes at ... " when I omit the removeShortArcs call. So, I'll try to run the first version of the patch against many tiles and see what happens. If I don't find a problem in Europe, it is likely that we don't need iteration. OK? 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
On 17.01.2013 16:01, Gerd Petermann wrote:
reg. test data: I did not yet try to understand why a short arc brakes routing. I assume it is because you can't calculate the bearing with an appropriate precision? I just know that I see error messages like "road ... contains zero length arc at ... " or "road ... contains consecutive identical nodes at ... " when I omit the removeShortArcs call. So, I'll try to run the first version of the patch against many tiles and see what happens. If I don't find a problem in Europe, it is likely that we don't need iteration.
OK?
Gerd
The reason As far as I can remember is that if we have the following situation # # # --- O UUU --- # # # Now consider both # and - U and O stand for streets. O must not be shorter than the routing resolution (which is twice the minimal distance we can put points), else O will be unroutable. I found out about it, by having a very short stretch of a road on a bridge, where directly after the bridge (or was is actually sharing a point with the bridge?) a crossing pops up. As soon as we made sure that O is ~5.4m the routing worked again. Im not sure if --- UUU --- with UUU being shorter than 5.4 works though. (UUU being e.g a bridge).
Hi Felix, I am not sure if I got that right. You use --remove-short-arcs=5.4 and you say that routing was broken unless the way was longer than 5.4 ? That sounds to me like the problem that I've described here: http://gis.19327.n5.nabble.com/short-roads-with-two-nodes-tp5744911.html Gerd Felix Hartmann-2 wrote
On 17.01.2013 16:01, Gerd Petermann wrote:
reg. test data: I did not yet try to understand why a short arc brakes routing. I assume it is because you can't calculate the bearing with an appropriate precision? I just know that I see error messages like "road ... contains zero length arc at ... " or "road ... contains consecutive identical nodes at ... " when I omit the removeShortArcs call. So, I'll try to run the first version of the patch against many tiles and see what happens. If I don't find a problem in Europe, it is likely that we don't need iteration.
OK?
Gerd
The reason As far as I can remember is that if we have the following situation
# # # --- O UUU --- # # #
Now consider both # and - U and O stand for streets. O must not be shorter than the routing resolution (which is twice the minimal distance we can put points), else O will be unroutable. I found out about it, by having a very short stretch of a road on a bridge, where directly after the bridge (or was is actually sharing a point with the bridge?) a crossing pops up. As soon as we made sure that O is ~5.4m the routing worked again.
Im not sure if
--- UUU --- with UUU being shorter than 5.4 works though. (UUU being e.g a bridge).
_______________________________________________ mkgmap-dev mailing list
mkgmap-dev@.org
-- View this message in context: http://gis.19327.n5.nabble.com/Test-failure-in-r2448-tp5744543p5745154.html Sent from the Mkgmap Development mailing list archive at Nabble.com.
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
On 17/01/13 20:17, Gerd Petermann wrote:
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.
OK I will commit the simple test fixes, and disable the complicated to fix one for now.
I still don't understand why a short arc breaks routing, I assume this is caused by some Garmin limit.
We know that short arc are problematic, but not exactly why. There are a few possibilities. 1. The routing arcs have a length field, when the arc length is less than 4.9m (in our implementation) this is stored as 0. Perhaps zero is bad and using 1 instead would work. 2. Perhaps the nodes must have different Garmin coordinates. This would imply a minimum separation of 2.3 (or less depending on direction and latitude). 3. I believe that boundary nodes between two maps are considered connected if they are up to 2 (or 3, I forget) garmin units apart. Perhaps this is true in general and routing will jump to unconnected roads if nodes on nearby roads are too close. (I thought of that last in relation to removeShortArcs removing complete ways - how did no one notice? Well perhaps because very small gaps can be jumped). ..Steve
Hi Steve,
We know that short arc are problematic, but not exactly why. There are a few possibilities.
1. The routing arcs have a length field, when the arc length is less than 4.9m (in our implementation) this is stored as 0. Perhaps zero is bad and using 1 instead would work.
2. Perhaps the nodes must have different Garmin coordinates. This would imply a minimum separation of 2.3 (or less depending on direction and latitude).
3. I believe that boundary nodes between two maps are considered connected if they are up to 2 (or 3, I forget) garmin units apart. Perhaps this is true in general and routing will jump to unconnected roads if nodes on nearby roads are too close.
A lot of maybes :-( I have one more: I think Garmin also changes the algorithmn with new firmware (one version broke routing completely on my Oregon 450t, don't remember exactly which one it was)
(I thought of that last in relation to removeShortArcs removing complete ways - how did no one notice? Well perhaps because very small gaps can be jumped).
The small way is removed, but the gpx device will probably still route you through it (if my last description of the algo is correct) I am more concerned that it may remove a way with highway=steps and connect the nodes of two big street directly instead. Gerd
participants (4)
-
Felix Hartmann -
Gerd Petermann -
GerdP
-
Steve Ratcliffe