Add support for ignoring live traffic closures for waypoints #2685

mandeepsandhu · 2020-11-17T04:30:12Z

Adds support for ignoring road closures due to traffic for location
waypoints, by way of a new search_filter option - "exclude_closures"

Roads which have live traffic speed of 0, are considered closed. If
search_filter.exclude_closures is set to false for a location, then any
outgoing edges that are marked closed, will be considered when loki
searches for start/end candidates.

kevinkreiser · 2020-11-18T21:48:59Z

valhalla/sif/dynamiccost.h

+   * @param  edgeid         GraphId of the opposing edge.
+   * @return  Returns true if the edge is filtered due to live traffic closure, false if not.
+   */
+  inline virtual bool FilterClosed(const baldr::DirectedEdge* edge,


im not a fan of making this public especially bcause it has some hidden state in there making use of filter_closures_

what if we make a generic public function like this:

inline virtual bool IsClosed(const baldr::DirectedEdge* edge, const baldr::GraphTile* tile) const { return (flow_mask_ & baldr::kCurrentFlowMask) && tile->IsClosed(edge); }

and inside of the Filter function we call it with the boolean filter_closures_ and inside of the allowed function we call it with the boolean ignore_closures.

I don't see the Allowed function using ignore_closures currently. Looks like its used during a-star search? Not sure of its impact if we move the ignore_closures check there.

Even with that, the basic issue IMO, still stands i.e during search (specifically in is_search_filter_triggered), how do we know the state of ignore_closures? filter_closures is already appropriately set taking ignore_closures into account, so I thought it would be good to expose it indirectly via the Filter function. I don't think doing so exposes an internal state, but rather rather exposes an API that just uses it.

Not sure how making IsClosed public helps here, since we still need to access filter_closures_ during search. Maybe I'm missing something.

Actually, I don't need to make FilterClosed (or IsClosed) public. I guess I can call costing.Filter(...) inside is_search_filter_triggered:

if ((road_class > min_road_class || road_class < max_road_class) || (search_filter.exclude_tunnel_ && edge->tunnel()) || (search_filter.exclude_bridge_ && edge->bridge()) || (search_filter.exclude_ramp_ && (edge->use() == Use::kRamp)) || (search_filter.exclude_closures_ && costing.Filter(edge, tile))) { return true;

Here's how I think it will work:

ignore_closures: false, exclude_closures: true (both defaults): Costing::Filter filters candidates due to closures Search filter is never called for candidates that are already removed by Costing::Filter.

ignore_closures: true, exclude_closures: true or false: This sets filter_closures_ to false and Costing::Filter does not filter any candidates due to closures. search filter is also not triggered, since Costing::Filter always returns 0.0/false

ignore_closures: false, exclude_closures: false: This sets filter_closures_ to false and Costing::Filter does not filter any candidates due to closures. search filter is also not triggered, since exclude_closures is false.

kevinkreiser · 2020-11-18T22:01:32Z

src/worker.cc

+    // set the corresponding internal costing attribute, so that loki search
+    // does not drop a start/end candidate when an edge is marked as closed due
+    // to live traffic


NIT:

Suggested change

// set the corresponding internal costing attribute, so that loki search

// does not drop a start/end candidate when an edge is marked as closed due

// to live traffic

// we tell the costing to let all closed roads through so that we can do

// a secondary per-location filtering using lokis search_filter functionality

CHANGELOG.md

mandeepsandhu · 2020-11-19T21:11:29Z

src/worker.cc

@@ -825,7 +845,6 @@ void from_json(rapidjson::Document& doc, Options& options) {

  // Parse all of the costing options in their specified order
  sif::ParseCostingOptions(doc, "/costing_options", options);
-  options.set_costing(costing);


Removed, since its being set above:

if (valhalla::Costing_Enum_Parse(costing_str, &costing)) { options.set_costing(costing); } else { throw valhalla_exception_t{125, "'" + costing_str + "'"}; }

kevinkreiser · 2020-11-20T21:16:15Z

src/loki/search.cc

@@ -36,7 +38,8 @@ bool is_search_filter_triggered(const DirectedEdge* edge,
  if ((road_class > min_road_class || road_class < max_road_class) ||
      (search_filter.exclude_tunnel_ && edge->tunnel()) ||
      (search_filter.exclude_bridge_ && edge->bridge()) ||
-      (search_filter.exclude_ramp_ && (edge->use() == Use::kRamp))) {
+      (search_filter.exclude_ramp_ && (edge->use() == Use::kRamp)) ||
+      (search_filter.exclude_closures_ && !costing.Filter(edge, tile))) {


it doesnt make sense for us to rely on the meaning of the costing filter here. for starters, this code path could never be hit if !costing.Filter is true because it means we would have filtered the edge out in the first place and never checked the search filters for it.

I suggest we just check if you want to exclude closures and if you are a costing that cares about current flow and current flow is 0. ie this:

Suggested change

(search_filter.exclude_closures_ && !costing.Filter(edge, tile))) {

(search_filter.exclude_closures_ && (costing.flow_mask() & baldr::kCurrentFlowMask) && tile->IsClosed(edge))) {

for starters, this code path could never be hit if !costing.Filter is true because it means we would have filtered the edge out in the first place and never checked the search filters for it

You're right, in that is_search_filter_triggered is never called, IF exclude_closures is true. However, if exclude_closure is false, we internally make filter_closures: true which results in !costing.Filter returning false and we end up invoking the search filter.

Basically there is never a case where we want Costing::Filter() to pass a candidate which might be filtered due to closures, but then have is_search_filter_triggered filter it out.

The change you suggested won't work if ignore_closures: true and exclude_closures: true, because the candidates will pass Costing::Filter but then fail in is_search_filter_triggered. So either we access the state of ignore_closures here, or use DynmaicCost::IsClosed by making it public (as that already takes ignore_closures into account).

I'm leaning towards using the ignore_closures value from the costing options pbf. Something like:

search_filter.exclude_closures = !costing_options.ignore_closures && location_options.exclude_closures

and then use it the way you suggested in is_search_filter_triggered. What do you think?

i guess im just saying that it makes no sense to do:

search_filter.exclude_closures_ && !costing.Filter(edge, tile)

this code says that if i want to exclude closures and the edge wasnt filtered before filter it now. it not being filtered before is not the same as saying it currently has a closure. there are many reasons an edge can be filtered, a closure is not the only reason.

let me think about this a bit more. there simply has to be a way to make this less confusing 😄

i guess im just saying that it makes no sense to do:

search_filter.exclude_closures_ && !costing.Filter(edge, tile)

this code says that if i want to exclude closures and the edge wasnt filtered before filter it now. it not being filtered before is not the same as saying it currently has a closure. there are many reasons an edge can be filtered, a closure is not the only reason.

Understood. We don't have to use costing filter here. Thats why I suggested accessing ignore_closures from the costing options here (or make DynmaicCost::IsClosed public). We'll need it to respect its override behavior.

mandeepsandhu · 2020-12-04T01:12:05Z

bench/thor/routes.cc

@@ -81,7 +82,8 @@ boost::property_tree::ptree build_config(const char* live_traffic_tar) {

 constexpr float kMaxRange = 256;

-static void BM_UtrechtBidirectionalAstar(benchmark::State& state) {
+template <class ALGO>


Updated benchmark test to be able to run multiple algo's for the same test.

kevinkreiser · 2020-12-04T21:20:46Z

@mandeepsandhu do you think we should consider an alternate implementation where we treat closures at the beginning and ending of routes similar to not_through regions. ie we can route to get out of them but we dont go in them in the middle of the route? i worry that a user getting a different edge candidate simply because they snapped to a multi edge closure would be unexpected/desired. was breifly discussing this with @danpaz

docs/api/turn-by-turn/api-reference.md

danpaz · 2020-12-04T23:24:24Z

src/worker.cc

@@ -179,6 +180,8 @@ const std::unordered_map<unsigned, std::string> OSRM_ERRORS_CODES{
     R"({"code":"InvalidValue","message":"The successfully parsed query parameters are invalid."})"},
    {142,
     R"({"code":"InvalidValue","message":"The successfully parsed query parameters are invalid."})"},
+    {143,
+     R"({"code":"InvalidValue","message":"The successfully parsed query parameters are invalid."})"},


Can this error message specify what was invalid about the query?

Once we can confirm that is the case and those test cases are failing, we can disable them and get thsi shipped with the idea that we will do smoe more work. specifically i think we should make starting and ending in a closure work like starting/ending in a not_thru area, where you are allowed to route inside of it until you leave it. as long as your predicessory label is marked that way you can keep expanding in there but as soon as the predecessor is not marked that way you can no longer expand edges like that. this will require us to know whether or not an edge is closed outside of the allowed method though which will be quite annoying...

I get how continuing to expand on consecutive closures would work when doing bidir a-star (since we start expansion from both ends). But what would the behavior be for the unidir a-star variants if there are multiple consecutive closures leading to the destination? When expanding from an intermediate open edge, we would avoid the closure, even though it might lead to the destination. If so, would it fail to find the route (what would happen if this was a not-thru edge instead)?

i cant remember if the forward/reverse-only algos handle this, i have to check the code to refresh my memory on that

Can this error message specify what was invalid about the query?

The specific error message is described in the new internal error code. I think the error messages here are kept to match OSRM, which has this message for most error codes (@kevinkreiser CMIIW).

It's unfortunate though, since when making a request with format=osrm we lose any context the error message might have had. No change needed for this PR though.

danpaz · 2020-12-04T23:32:41Z

test/gurka/test_search_filter.cc

+    gurka::assert::raw::expect_path(result, {"HIC", "CFGD"});
+
+    // Specify search filter to disable exclude_closures at departure. Due to
+    // consecutive closures, this should result in a "no route"


I think this test captures the multi-edge closure case, which we do want to return a valid route.

For completeness we should also try a test that has closures not just at the origin and destination but also on edge in between, to ensure we allow closures at the endpoints but still avoid them along the route.

For completeness we should also try a test that has closures not just at the origin and destination but also on edge in between, to ensure we allow closures at the endpoints but still avoid them along the route.

Intermediate closure scenario is covered in IgnoreClosuresOverridesExcludeClosures testcase. We route from 1 -> 2 and close edge CD. Without ignore_closures costing option we don't route through CD which is an intermediate edge.

Would you rather see this a separate standalone test (and named appropriately)?

I was thinking of adding a new test case that closes the origin edge, destination edge, and an intermediate edge, to exercise the case where a user can snap to a closure but avoid closures in between the snapped edges. Maybe this logic path is covered already though?

I think this test captures the multi-edge closure case, which we do want to return a valid route.

That would depend on whether there are suitable edges nearby, no? If loki's search results in only 1 candidate edge, then I guess if there are no reachable edges to it it would fail to find a route? CMIIW.

But in this test the request has exclude_closures=false, so we know that there are suitable edges to snap to. Why then does it return a NoRoute?

kevinkreiser · 2020-12-08T21:56:21Z

i think to get this to a point where we can ship id like to offer up the following idea. we add 2 test cases where we set filter_closures:false, they should look something like:

where there is a multiple edge closure but we set reachability so that the edge in the closure is not reachable but an edge outside of it is, creating a long snap anyway and routing around
where there is a multipe edge closure but we set a heading on the origin. the route will then hit the second edge and have to do a uturn to get around the closure

Once we can confirm that is the case and those test cases are failing, we can disable them and get thsi shipped with the idea that we will do smoe more work. specifically i think we should make starting and ending in a closure work like starting/ending in a not_thru area, where you are allowed to route inside of it until you leave it. as long as your predicessory label is marked that way you can keep expanding in there but as soon as the predecessor is not marked that way you can no longer expand edges like that. this will require us to know whether or not an edge is closed outside of the allowed method though which will be quite annoying...

mandeepsandhu · 2020-12-10T07:09:39Z

docs/api/turn-by-turn/api-reference.md

@@ -494,6 +495,7 @@ The codes correspond to code returned from a particular [Valhalla project](https
 |140 | Action does not support multimodal costing |
 |141 | Arrive by for multimodal not implemented yet |
 |142 | Arrive by not implemented for isochrones |
+|143 | ignore_closure in costing and exclude_closure in search_filter cannot both be specified |


This is the new exception type added which explains the error related to the new option.

mandeepsandhu · 2020-12-14T19:02:05Z

@kevinkreiser @danpaz I've added more tests for the following scenarios:

Closures on trivial routes (src/dst on same edge & adjacent edges which would fallback to using uni-directional a*)
Added tests with consecutive closures that can cause possible u-turns & distant edge snaps. Both tests are currently disabled.
Added ability to test with different costing models & 2 date_time type (0, 3). date type 0 is currently disabled since that uses timedep-fwd astar which does not work for closures at destinations.

CC: @danpat

mandeepsandhu · 2020-12-14T20:14:24Z

src/worker.cc

@@ -685,6 +720,43 @@ void from_json(rapidjson::Document& doc, Options& options) {
    options.set_linear_references(*linear_references);
  }

+  // costing defaults to none which is only valid for locate


This code block was moved up before parsing locations so that we have the costing options parsed before hand. This helps us do the following checks:

If ignore_closure is set, it overrides the default exclude_closure behavior.

If both ignore_closures & exclude_closure are set, we throw an exception back to the caller.

mandeepsandhu · 2020-12-14T20:17:55Z

test/gurka/test_search_filter.cc

+        H---I
+
+
+   L-4---5-M-6-N


Used for testing trivial cases which fallback to using timedep fwd

mandeepsandhu · 2020-12-14T20:34:59Z

test/gurka/test_search_filter.cc

+    };
+    test::customize_live_traffic_data(closure_map.config, close_edge);
+
+    // TODO: 1->2 fails to get route without ignoring closure at 2. why?


I found it odd that if I close edge DE and route to 2, its unable to find the route. However the same does not happen when I close JK & route to 3. I haven't found a good reason for this difference in behavior yet.

CC: @kevinkreiser @danpat @danpaz

@mandeepsandhu I wonder if it's node snapping since DE is shorter than JK. Can you try increasing the gridsize from 10 to 100 and see if the same happens?

Changing the gridsize to 100 does stop the no route exception. When you say "node snapping", do you mean its snapping to E? If so, shouldn't it still route upto D?

Actually I also had to increase DE's length, otherwise its still unable to find the route

test/gurka/test_search_filter.cc

mandeepsandhu · 2020-12-14T20:57:04Z

test/gurka/test_search_filter.cc

+            .str();
+    result = gurka::route(closure_map, req_disable_exclude_closures, reader);
+    gurka::assert::osrm::expect_steps(result, {"CD", "AC", "AB", "BE"});
+    gurka::assert::raw::expect_path(result, {"CD", "CD", "AC", "AB", "BE"});


This test currently fails if expecting a uturn on CD (the actual path returned is "CD", "AC", "AB", "BE") even with a heading of 90º (going towards D).

Seems like the uturn wouldn't occur with left-side driving?

mandeepsandhu · 2021-01-08T07:05:39Z

Performance impact of change:

branch:         |            50th              |             90th             |         99th
master          | 0.03990912437438965          | 0.07622202237447102          | 0.19320416450500488
this branch     | 0.04096802075703939 (+2.65%) | 0.07817689577738444 (+2.56%) | 0.19673490524291992 (+1.83%)

Note: These are average of 3 runs for each branch with 20k route requests (pre-warmed tile-cache by running 20k requests before measuring performance of each branch).

Also, for another set of test requests (de_benchmark_routes.txt) I got an improvement in the 90th percentile of about 0.3%. But this seems t be within the margin of measurement noise. Although the 2.5% needs to be looked at. Thoughts @kevinkreiser ?

* Add support for ignoring live traffic closures for waypoints Adds support for ignoring road closures due to traffic for location waypoints, byt way of a new search_filter option - "exclude_closures" Roads which have live traffic speed of 0, are considered closed. If search_filter.exclude_closures is set to false for a location, then any outgoing edges that are marked closed, will be considered when loki searches for start/end candidates. * lint fix * an idea of how to have ignore_closures win over filter_closures when its turned on * Update filtering logic to override search_filter.exclude_closures if costing_options.ignore_closures is set * Reword api docs * Use Costing::Filter in is_search_filter_triggered and update changelog * typo * Cleanup and add more tests for conflicting options * Fixed astar algos to include destination closures. Broke prefferred side test * add expansion tracking for debugging purposes * Fix merge conflicts due to astar algo removal * lint fix * Add new internal exception code and update docs * lint fix * reorder top_speed & filter_closure in dynamiccost.h * More tests. Disable tests for consecutive closures * enable more costing profiles in tests * linting * clang-tidy fix * Address review comments * updated one of the disabled tests with comments on why its disabled * change to use intrusive ptr after merging latest master * modify the graph so that snapping happens where we expect * Added disabled consecutive closure test with reachability set * lint fix * remove the need to expose Costing::IsClosed and fix a bug in request parsing not overriding exclude_closure when only ignore_closures is set * rename filter to allowed to match the rest of the methods * refactor of filter function complete, now we can control how it works from loki::search and loki::reach * Track Distance in Isochrones and Generate Distance Contours Optionally (#2699) * templatize the value part of gridded data * remove templates and stub out for adding distance contours * [ci skip] **NO_CI** first implementation for isodistances; lacks distance tracker in siff::Cost * temp * distance for isos works now on top of time; no tests yet; no multimodal implementation yet * add to docs * address review comments; add path_distance to origin and destination edge labels * map orders internally.. anyways, switch to lighter syntax [ci skip] ***NO_CI*** * partial distance for origin/destination edges via directededges [ci skip] ***NO_CI*** * simplify isodistances addition by making gridded data class do more of the work * fix what must be a typo * update tests and test configs * bugs in dijkstras labels, still bugs in gridded data * typo in griddata, forgot to add initial feature to contour * all bugs squashed. performance checks remaining * clean up benching diffing server based tool. allow writing no output to disk. track progress more efficiently * fix benchmark configs * simplify isochrone limits check * reclaim performance loss * lose the ternaries as well * try to fix mac build * shave more dead code out of the gridded data interface * straighten out bug in max_time/distance to construct grid * satisfy linter * back to using std::max_element but this time the comparator is actually properly implemented as less than ;o) Co-authored-by: nilsnolde <nils.nolde@gmail.com> * optimizations (#2743) * reserve protobuf objects wherever possible in path building * add some reservations to the serialization code as well to derisk reallocs * convert serialization for valhalla format to rapidjson SAX writer * namespace missing * avoid hashing and indirection in simple tile cache * templatize double bucket queue to remove lambda calls. step on the way to merging edgelabel edgestatus and queue into one structure * fix build, maybe not tests * simplify derived uturn in bidirectional expansion * simplify expansion functions with less conditionals * remove recursion from expansion methods in standard algorithms * use route_index in serializer * allow using simple cache to avoid 8.5mb ram usage * clean up make_pair to have the std namespace prefix. not sure how it compiled without it... * cleanup * more cleanup * Enhance BSS import (#2575) * fix when projected edge's start node and end node are not in the same tile * refactor * split the bss connections for pedestrian and bicycle * refactor * update bssimport test * fix test * add test and use google test utils * remove accidentally added header * code cleaning after lint * fix test after merging master * fix windows compilation error; remove some uselss headers * lint * fix complilation bug * format Co-authored-by: Kevin Kreiser <kevinkreiser@gmail.com> * remove filter function and do filtering inline so we can control it by parameter. update reach to make use of this control * refactor the common stuff in the allowed method to the base class * lint * tidy * tidy again * lint again Co-authored-by: Mandeep Sandhu <mandeep.sandhu@mapbox.com> Co-authored-by: nilsnolde <nils.nolde@gmail.com> Co-authored-by: Patrick Qian <patrick.qian@kisio.com>

kevinkreiser · 2021-01-11T17:28:06Z

bench/thor/routes.cc

@@ -11,6 +11,7 @@
 #include "sif/costfactory.h"
 #include "test.h"
 #include "thor/bidirectional_astar.h"
+#include "thor/timedep.h"


can we revert this file? i know this was a long running pr with many iterations but thsi seems irrelevant now?

I was using bench for seeing the impact of the change on timedep a-star implementations. We're no longer doing that in this PR now, so I could revert it. Although I feel its okay to test the timedep implementations in the bench tests for completeness.

kevinkreiser · 2021-01-11T17:29:00Z

run_route_scripts/run_with_server.py

@@ -93,7 +92,7 @@ def make_request(post_body):
  parser.add_argument('--test-file', type=str, help='The file with the test requests', required=True)
  parser.add_argument('--url', type=str, help='The url to which you want to POST the request bodies', default='http://localhost:8002/route')
  parser.add_argument('--output-dir', type=str, help='The directory in which to place the result of each request')
-  parser.add_argument('--concurrency', type=int, help='The number of processes to use to make requests', default=psutil.cpu_count(logical=True))
+  parser.add_argument('--concurrency', type=int, help='The number of processes to use to make requests', default=multiprocessing.cpu_count())


was this something mac specific or?

No. This happened on Linux as well. I didn't have psutil installed so this script was failing. Since multiprocessing has the cpu_count I thought of using that instead of psutil since its already in use here.

i mean you can just pip install it but sure, one less thing seems better (i cant remember why i used that in the first place, maybe one works better under docker or something...)

yeah 1 less package to install. I _ think_ this should work inside docker too.

kevinkreiser · 2021-01-11T17:33:27Z

test/gurka/test_instructions_small_end_ramp_forks.cc

@@ -62,7 +62,7 @@ class InstructionsSmallEndRampForks : public ::testing::Test {
    const auto layout =
        gurka::detail::map_to_coordinates(ascii_map, gridsize_metres, {5.1079374, 52.0887174});

-    map = gurka::buildtiles(layout, ways, {}, {}, "test/data/gurka_instructions_keep_highway_signs",
+    map = gurka::buildtiles(layout, ways, {}, {}, "test/data/gurka_instructions_small_end_ramp_forks",


this was a collision with another test, looks like it was a copy paste fail

…e unused function from dynamiccost

kevinkreiser

PR looks clean to me now. Sorry for the long slog on this one!

mandeepsandhu · 2021-01-11T18:26:16Z

PR looks clean to me now. Sorry for the long slog on this one!

Thanks for slogging with me on this one! :) Appreciate you taking the time to help me along the way 🙏

mandeepsandhu · 2021-01-11T22:18:43Z

Performance impact after latest merge from master:

branch:         |            50th              |             90th             |         99th
master          | 0.03852168718973795          | 0.07410685221354167          | 0.18502410252888998
this branch     | 0.03876058260599772 (+0.62%) | 0.07470011711120605 (+0.8%)  | 0.18315505981445312 (*-1.01%*)

Note: These are average of 3 runs for each branch with 20k route requests (pre-warmed tile-cache by running 20k requests before measuring performance of each branch).

RAD tests showed 0 diffs. I think we're good to merge this change.

mandeepsandhu marked this pull request as draft November 17, 2020 04:30

mandeepsandhu force-pushed the ignore_closures branch from 2d49e0c to 8a8c834 Compare November 18, 2020 21:21

mandeepsandhu marked this pull request as ready for review November 18, 2020 21:21

mandeepsandhu requested review from kevinkreiser and danpaz November 18, 2020 21:21

kevinkreiser reviewed Nov 18, 2020

View reviewed changes

mandeepsandhu force-pushed the ignore_closures branch from dfa33a2 to c94c782 Compare November 19, 2020 04:50

danpaz reviewed Nov 19, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

mandeepsandhu commented Nov 19, 2020

View reviewed changes

kevinkreiser reviewed Nov 20, 2020

View reviewed changes

mandeepsandhu force-pushed the ignore_closures branch from f3f3ddf to e0dcaf9 Compare December 4, 2020 01:10

mandeepsandhu commented Dec 4, 2020

View reviewed changes

danpaz reviewed Dec 4, 2020

View reviewed changes

mandeepsandhu force-pushed the ignore_closures branch from 29f2e7d to ca18da4 Compare December 10, 2020 07:05

mandeepsandhu commented Dec 10, 2020

View reviewed changes

mandeepsandhu force-pushed the ignore_closures branch 2 times, most recently from f883d1d to a1cc150 Compare December 14, 2020 18:56

mandeepsandhu force-pushed the ignore_closures branch from a1cc150 to d2cd831 Compare December 14, 2020 20:08

mandeepsandhu requested review from danpat, danpaz and kevinkreiser December 14, 2020 20:08

mandeepsandhu commented Dec 14, 2020

View reviewed changes

test/gurka/test_search_filter.cc Show resolved Hide resolved

mandeepsandhu commented Dec 14, 2020

View reviewed changes

mandeepsandhu and others added 13 commits January 7, 2021 21:49

Add new internal exception code and update docs

3d45bf8

lint fix

4ee3db3

reorder top_speed & filter_closure in dynamiccost.h

9fbfaef

More tests. Disable tests for consecutive closures

96275ce

enable more costing profiles in tests

5841620

linting

78edd59

clang-tidy fix

cddf4ab

Address review comments

1778e1e

updated one of the disabled tests with comments on why its disabled

28891b5

change to use intrusive ptr after merging latest master

7a0502f

modify the graph so that snapping happens where we expect

189c29a

Added disabled consecutive closure test with reachability set

32a4983

lint fix

0dc7df6

mandeepsandhu force-pushed the ignore_closures branch from 2430dbf to 0dc7df6 Compare January 8, 2021 05:50

kevinkreiser mentioned this pull request Jan 8, 2021

Fix Reach for Exclude Closures #2765

Merged

kevinkreiser and others added 3 commits January 11, 2021 11:22

Merge branch 'master' into ignore_closures

862c8b2

remove clashing directory name with other test

13b7261

kevinkreiser reviewed Jan 11, 2021

View reviewed changes

kevinkreiser added 3 commits January 11, 2021 12:45

rever benchmark, clean up use of filter function to be boolean, remov…

1d9080f

…e unused function from dynamiccost

remove duplicate changelog entry

cebe1d2

remove duplicate changelog entry

a1cd0f7

kevinkreiser approved these changes Jan 11, 2021

View reviewed changes

kevinkreiser merged commit bc76e60 into master Jan 11, 2021

purew deleted the ignore_closures branch January 13, 2021 19:14

	(search_filter.exclude_closures_ && !costing.Filter(edge, tile))) {
	(search_filter.exclude_closures_ && (costing.flow_mask() & baldr::kCurrentFlowMask) && tile->IsClosed(edge))) {

Add support for ignoring live traffic closures for waypoints #2685

Add support for ignoring live traffic closures for waypoints #2685

Conversation

mandeepsandhu commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandeepsandhu Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser Nov 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser commented Dec 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser commented Dec 8, 2020

Choose a reason for hiding this comment

mandeepsandhu commented Dec 14, 2020

mandeepsandhu Dec 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandeepsandhu commented Jan 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinkreiser left a comment

Choose a reason for hiding this comment

mandeepsandhu commented Jan 11, 2021

mandeepsandhu commented Jan 11, 2021

mandeepsandhu Nov 19, 2020 •

edited

Loading

kevinkreiser Nov 21, 2020 •

edited

Loading

kevinkreiser Dec 9, 2020 •

edited

Loading

mandeepsandhu Dec 14, 2020 •

edited

Loading