[mkgmap-dev] [PATCH V4] boundary preparer with quadtree

Mon Feb 20 12:50:00 GMT 2012

Hi WanMil,

thanks for the quick response.

> I've just started to look through the sources. I haven't read all so 
> maybe some of my comments are just wrong...

I think all are okay... 

> 
> * class BoundaryLevelCollator in BoundaryLocationPreparer:
> I think you have copied that from the LocationHook. But now you are 
> using it in a different context which might cause problems. The collator 
> retrieves the name of an admin_level and compares them. In the 
> LocationHook it can use the final name-tag-list but during the 
> preparation step the preparer does not know anything about the 
> name-tag-list (it is possible that I prepare the bounds with 
> "name:de,name:it" and you are using the prepared bounds with "name:zh"). 
> The chances are not very high that this causes a problem but I think 
> during preparation you should check all tags matching "*name*" to be safe.

I did already think about this as well, and in fact today I've removed that part from 
BoundaryLocationPreparer and changed BoundaryQuadTree to use always the
AdminLevelCollator() method and sort the input with that. 
This one ignores the name, I think.
That solved some of the problems in BoundaryMerger. 
Regarding the name tag: The method BoundaryElementSaver.isBoundary() 
can remove bounaries that do not have the name tag. This might also be a problem.

> * Good idea to use the max-jobs parameter for the preparation. Maybe you 
> can use the threadPool ExecutorService from the Main class instead of 
> creating your own thread environment? Implementing the preparer as a 
> thread was not my final idea but it was easy to realize so I didn't 
> invest the time to change that.

Yes, I think this can be done. We have to split the preparer, so that the 1st pass is executed as 
a single thread.

> 
> * FindBugs found two warnings:
> BoundaryQuadTree
> line 1048:
>    if (nodes.get(i).boundaryId == nodes.get(i-1).boundaryId){
> line 1271:
>    if (o1 == o2) {
> 
> Strings should be compared with equals instead of ==
> 
OK, I will change that.

The problem in the BoundaryMerger is solved, it did not produce a wrong result, it just 
created some deeper sub-trees and therefor had to write more areas. 
The new version does the oposite: It typically creates smaller trees and smaller output
files, so you can think of it as an optimizer :-)

I'll post a new patch today.

Gerd

> That's it for now.
> 
> WanMil
> 
> > Hi Gerd,
> >
> > thanks for that patch. It's a big step forward :-)
> > I've comitted it and also merged all changes from the trunk so that the
> > branch does not diverge too much.
> >
> > I tried to compile boundaries for an old planet file but got an
> > exception in the BoundaryMerger:
> > Exception in thread "main" java.lang.NullPointerException
> >           at
> > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryMerger.merge(BoundaryMerger.java:93)
> >           at
> > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryMerger.main(BoundaryMerger.java:144)
> >
> > Can you have a look on it?
> > Thanks!
> >
> > WanMil
> >
> >> Hi WanMil,
> >>
> >> attached is the new patch for the performance branch.
> >> Its quite big because I've rewritten many methods to work with the quadtree
> >> instead of
> >> List<boundary>
> >>
> >> Major changes:
> >> - new file format for *.bnd files. It stores first the boundary tags, then
> >> the areas with float precision.
> >> The file size is not much different from the old format, but when zipped it
> >> is much smaller.
> >> The time to load the file into the quadtree is shorter than the time to load
> >> the old format.
> >> The old format is still supported, but requires much more time in the
> >> LocationHook (probably it is still
> >> faster than trunk, but not much)
> >>
> >> - the bounds parameter allows now to specify a zip file with the *.bnd
> >> files. The zip file can have a directory structure, but should not contain
> >> duplicate *.bnd files.
> >> - BoundaryPreparer uses multiple processors for the
> >> workoutBoundaryRelations() part when --max-jobs parameter allows it. When
> >> called as stand-alone program, it starts one thread for each processor.
> >>
> >> - the utilities BoundaryDiff, BoundaryMerger, BoundaryFile2Gpx,
> >> BoundaryLister were rewritten to use the quadtree, most of them also allow
> >> *.zip as input. BoundaryLister lists only the OSM tags, not the
> >> information created for the quadtree. Can be changed if needed.
> >>
> >> Open problem:
> >> The BoundaryMerger creates a result that contains a few more small holes
> >> than the trunk version. I'm going to analyse that during the next days.
> >> Todo: Javadoc
> >>
> >> Ciao,
> >> Gerd
> >>
> >>
> >> http://gis.19327.n5.nabble.com/file/n5495676/boundary_prep_quadtree_v4.patch
> >> boundary_prep_quadtree_v4.patch
> >>
> >> --
> >> View this message in context: http://gis.19327.n5.nabble.com/PATCH-V4-boundary-preparer-with-quadtree-tp5495676p5495676.html
> >> Sent from the Mkgmap Development mailing list archive at Nabble.com.
> >> _______________________________________________
> >> mkgmap-dev mailing list
> >> mkgmap-dev at lists.mkgmap.org.uk
> >> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> >
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev at lists.mkgmap.org.uk
> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> 
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20120220/477bf160/attachment.html 