avoid polygons

[nilsnolde]

fixes #2570

based on #2744.

In the request specify avoid_polygons with multiple rings (actually MultiPolygon without inner rings..), e.g. avoid_polygons=[[[0,0],[0,1],[1,1],[1,0],[0,0]]].

Tasklist

• avoid_polygons in request parameters
• actual filtering of AvoidEdges

[nilsnolde]

hmm.. I think geographic area calculation might not be supported for boost.geometry 1.65 in circle ci.. Not sure how to fix this, tried to specify a geographic strategy.. Would be nice to be able to get polygon/ring area easily.

@mloskot maybe you have an idea (sometime after xmas;)))? https://github.com/valhalla/valhalla/blob/75ecbdd54b44dc5a8caf82e0fbca777d9a928258/src/loki/polygon_search.cc

edit: yeah, the strategy::area stuff seems to be pretty volatile in bg..

[mloskot]

@nilsnolde Given the amount of fixes and additions w.r.t. geographic calculations the Boost.Geometry received over last years, the 1.65 is pretty ancient, but I can not tell for sure. Is this still a probolem?

Looks like the latest failures are

/root/project/src/worker.cc: In function 'void {anonymous}::from_json(rapidjson::Document&, valhalla::Options&)':
/root/project/src/worker.cc:894:3: error: 'parse_polygons' was not declared in this scope; did you mean 'parse_locations'?
  894 |   parse_polygons(doc, options, "avoid_polygons", 137);

[kevinkreiser]

im guessing the only thing holding us on an older version of boost is the x86 build, since we cant get newer x86 ubuntu images. perhaps we should just retire the x86 build. we could consider bringing in an arm build in its place that targets a 32bit arm architecture. That seems to be the only modern architecture where i would expect to see devices in the wild that might have to run a 32bit build of valhalla.

[nilsnolde]

Looks like the latest failures are

yeah I pushed smth pre-mature, apparently forgot to change one call.. I think I'll leave out area calculation anyways, as it turns out the better metric here is circumference. I'll see today or tmrw what CI thinks about that..

The old versions complicate dev too.. Guess I'll have to switch developing in a ubuntu container (on arch right now). Anyways, bumping versions would be nice of course. Did you ever try an arm build with valhalla @kevinkreiser ? Wondering if all dependencies are packaged at least..

[nilsnolde]

finally works!

removing bg::area might've done the trick for boost.geometry as well I hope.. few todos left, but almost there 🍾 still not careful enough with left-overs from other branches, lots of non-sense files..

[kevinkreiser]

@nilsnolde since we removed the x86 builds i think we should be safe to update to whatever boost we have in ubuntu 20.04 and whatever version of osx we are running with brew. we should probably do that in a separate pr as well

[nilsnolde]

Ok sounds good @kevinkreiser . CI seems to only fail on some stupidity I added in one test. A new boost version shouldn’t be a blocker for this PR at least.

[nilsnolde]

Thanks for the further review @kevinkreiser! Think I could change it all as you recommended.. Let me know when you looked at the core functionality..

[kevinkreiser]

this looks done to me. i just added a couple of tiny nits to the main algorithm. as soon as you resolve those ill drop a ship here!

[nilsnolde]

cool thanks @kevinkreiser. done with addressing the comments.

[nilsnolde]

I stopped CI, will merge #2752 first, then it'll run here again

[purew]

Just flagging that the unittests in this PR fail on newest Clang

[          ] Valhalla request is: {"locations":[{"lon":0.00017966240894892044,"lat":0.0},{"lon":0.00017966240894892044,"lat":-0.0005389
872268353854}],"costing":"auto","exclude_polygons":[[[13.38625361,52.4652558],[13.38625361,52.48000128],[13.4181769,52.48000128],[13.41
81769,52.4652558]]]}                                                                                                                   
[       OK ] AvoidTest.TestMaxPolygonPerimeter (11 ms)             
[ RUN      ] AvoidTest.TestAvoidShortcutsTruck                                                                                         
../test/gurka/test_avoids.cc:213: Failure                        
Expected equality of these values:                                                                                                     
  found_shortcuts                                                                                                                      
    Which is: 0                                                                                                                        
  2                                                                                                                                    
[  FAILED  ] AvoidTest.TestAvoidShortcutsTruck (5 ms)              
[----------] 2 tests from AvoidTest (17 ms total)

I haven't dug deeper into why this fails

clang++ --version
clang version 12.0.0
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

[nilsnolde]

I don't see failing tests in the snippet.. sure you pasted the right part @purew ?

this particular test TestAvoidPolygonWithDeprecatedParam was added only fairly recently with #3093 , though also by me..

[purew]

Woops, updated the snippet above.