truck penalty for hgv=no edges #4650

nilsnolde · 2024-03-22T14:04:48Z

based on #4642

added hgv_no_access_penalty costing option in the range [0.f, kMaxPenalty[
if kMaxPenalty we treat hgv=no edges the same as today for truck costing, i.e. we restrict access
if < kMaxPenalty we:
- don't apply the penalty to correlated edge with hgv=no
- only start applying the penalty after it expands on its first hgv=yes

so in the end it works similar to destonly pruning, also with a penalty, only there’s no pruning of turned on.

nilsnolde · 2024-03-22T14:05:18Z

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

@@ -148,6 +148,7 @@ The following options are available for `truck` costing.
 | `axle_load` | The axle load of the truck (in metric tons). Default 9.07. |
 | `axle_count` | The axle count of the truck. Default 5. |
 | `hazmat` | A value indicating if the truck is carrying hazardous materials. Default false. |
+| `hgv_no_access_penalty` | A penalty applied to roads with no HGV/truck access. If set to a value less than 43200 seconds, HGV will be allowed on these roads and the penalty applies. Default 43200, i.e. trucks are not allowed. |


open for a different name:)

i dont know of another way to do it but i dislike having to document implementation details like the max penalty. it removes the ability for us to change it in the future which i also know would be a jerk move but still like to have the option for it to not be set in stone. anyway not asking you to do something different, more thinking outloud in case something comes to someone else mind about this.

nilsnolde · 2024-03-22T14:11:56Z

lua/graph.lua

@@ -91,7 +91,7 @@ access = {
 ["forestry"] = "false",
 ["destination"] = "true",
 ["customers"] = "true",
-["official"] = "false",
+["official"] = "true",


AFAIU official and permissive are signifying allowed access

yeah i think it would be easy for us to have messed this up by reading it as "official use only" which is a special U.S.-centric thing that means basically cops and emergency vehicles can use it but no one else. good catch

nilsnolde · 2024-03-22T14:12:15Z

proto/options.proto

@@ -313,6 +313,9 @@ message Costing {
    float use_lit = 82;
    bool disable_hierarchy_pruning = 83;
    bool ignore_non_vehicular_restrictions = 84;
+    oneof has_hgv_no_access_penalty {


has a default of kMaxPenalty

would have be cool to have the default be 0, not use oneof and then just had 0 mean not allowed but i guess there is a tiny argument for you to be able to say its both allowed and the penalty is actually 0. anyway would have allowed us to not document the internal max penalty or been able to pick a smaller soft limit or something i dont i know haha

nilsnolde · 2024-03-22T14:12:54Z

src/mjolnir/pbfgraphparser.cc

@@ -4990,7 +4990,8 @@ void PBFGraphParser::ParseRelations(const boost::property_tree::ptree& pt,
                                                        OSMPBF::Interest::CHANGESETS),
                          callback);
  }
-  LOG_INFO("Finished with " + std::to_string(osmdata.restrictions.size()) + " simple restrictions");
+  LOG_INFO("Finished with " + std::to_string(osmdata.restrictions.size()) +
+           " simple turn restrictions");


"restrictions" is a bit too generic IMO

nilsnolde · 2024-03-22T14:13:00Z

src/mjolnir/restrictionbuilder.cc

@@ -752,7 +752,7 @@ void RestrictionBuilder::Build(const boost::property_tree::ptree& pt,
    std::vector<std::promise<Result>> promises(threads.size());

    // Start the threads
-    LOG_INFO("Adding Restrictions at level " + std::to_string(tl->level));
+    LOG_INFO("Adding complex turn restrictions at level " + std::to_string(tl->level));


src/sif/truckcost.cc

nilsnolde · 2024-03-22T14:17:48Z

test/gurka/test_matrix.cc

+  std::unordered_map<std::string, std::string> cost_matrix =
+      {{"/costing_options/truck/hgv_no_access_penalty", "2000"},
+       {"/sources/0/date_time", "2024-03-20T09:00"},
+       {"/prioritize_bidirectional", "1"}};
+  std::unordered_map<std::string, std::string> td_matrix =
+      {{"/costing_options/truck/hgv_no_access_penalty", "2000"},
+       {"/sources/0/date_time", "2024-03-20T09:00"}};


making a time request makes sure we can call both costmatrix & tdmatrix in a simple loop below

nilsnolde · 2024-03-22T14:18:35Z

test/gurka/test_route.cc

+  const std::string ascii_map = R"(
+    A-1--B----C----D----E--2-F----G----H--3-I
+              |    |
+              J----K
+              |    |             
+              |    |
+              L----M
+           )";


basically the same map as for the matrix test

valhalla/sif/dynamiccost.h

src/sif/truckcost.cc

…d added tests for all algorithms except for dijkstra

nilsnolde · 2024-03-25T15:34:35Z

valhalla/sif/dynamiccost.h

-    if ((forward && (edge->end_restriction() & access_mode())) ||
-        (!forward && (edge->start_restriction() & access_mode()))) {
+    if ((forward && (edge->end_restriction() & access_mask_)) ||
+        (!forward && (edge->start_restriction() & access_mask_))) {


everything else uses access_mask_ instead of calling the public api and makes sense as long as we're inside the class

nilsnolde · 2024-03-25T15:40:49Z

ok, did the changes we talked about @kevinkreiser . I also saw that triplegbuilder uses costing->access_mode() to get the restrictions:

valhalla/src/thor/triplegbuilder.cc

Lines 1248 to 1253 in 962719a

    
           if (directededge->access_restriction() && restrictions_idx != kInvalidRestriction) { 
        
             const std::vector<baldr::AccessRestriction>& restrictions = 
        
                 graphtile->GetAccessRestrictions(edge.id(), costing->access_mode()); 
        
             trip_edge->mutable_restriction()->set_type( 
        
                 static_cast<uint32_t>(restrictions[restrictions_idx].type())); 
        
           }

which is fine though, we'd want them in the final trip edges for car mode as well if the new penalty was turned on.

kevinkreiser · 2024-04-02T01:19:33Z

src/sif/truckcost.cc

+
+  // determine what to do with hgv=no edges
+  bool no_hgv_access_penalty_active = !(costing_options.hgv_no_access_penalty() == kMaxPenalty);
+  no_hgv_access_penalty_ = no_hgv_access_penalty_active * costing_options.hgv_no_access_penalty();


nit: is this necessary, cant you just set it regardless? it will only be applied when the access doesnt have truck right? which wont happen anyway if its not "active" or am i missing something? just a nit not important

if we don't do it here and want to use it in transitioncost, then we'd have to check the same thing, i.e. if it's == kMaxPenalty. instead of having that ternary being done every time we call TransitionCost, I'd rather do it once here.

kevinkreiser · 2024-04-02T01:20:48Z

src/sif/truckcost.cc

@@ -551,6 +561,10 @@ Cost TruckCost::TransitionCost(const baldr::DirectedEdge* edge,
    c.cost += low_class_penalty_;
  }

+  // Penalty if the request wants to avoid hgv=no edges instead of disallowing
+  c.cost +=
+      no_hgv_access_penalty_ * (pred.has_hgv_access() && !(edge->forwardaccess() & kTruckAccess));


cool so we only penalize on entering it 👍

kevinkreiser · 2024-04-02T01:22:57Z

valhalla/sif/dynamiccost.h

+  /***
+   * Evaluates mode-specific and time-dependent access restrictions, including a binary
+   * search to get the tile's access restrictions.
+   *
+   * @param access_mode        The access mode to get restrictions for
+   * @param edge               The edge to check for restrictions
+   * @param is_dest            Is there a destination on the edge?
+   * @param tile               The edge's tile
+   * @param current_time       Needed for time dependent restrictions
+   * @param tz_index           The current timezone index
+   * @param restriction_idx    Records the restriction in the tile for later retrieval
+   */


kevinkreiser · 2024-04-02T01:24:20Z

valhalla/sif/edgelabel.h

+  uint32_t hgv_access_ : 1;
+  uint32_t spare : 12;


one day we'll just have the whole mask here and be done with it haha

nilsnolde · 2024-04-02T09:33:58Z

fyi, since the last master merge we only updated vicky's test files, it's safe to merge without running CI again.

nilsnolde commented Mar 22, 2024

View reviewed changes

src/sif/truckcost.cc Outdated Show resolved Hide resolved

nilsnolde commented Mar 22, 2024

View reviewed changes

src/sif/truckcost.cc Outdated Show resolved Hide resolved

nilsnolde commented Mar 22, 2024

View reviewed changes

valhalla/sif/dynamiccost.h Outdated Show resolved Hide resolved

nilsnolde commented Mar 22, 2024

View reviewed changes

valhalla/sif/dynamiccost.h Show resolved Hide resolved

nilsnolde requested review from dnesbitt61, kevinkreiser and gknisely March 23, 2024 19:45

nilsnolde commented Mar 25, 2024

View reviewed changes

src/sif/truckcost.cc Outdated Show resolved Hide resolved

nilsnolde added 6 commits March 25, 2024 16:13

some bug fixes ported over from #4638

f6fd150

minor fixes

38e0604

implemented hgv_no_access_penalty for all algorithms, updated docs an…

4c49d20

…d added tests for all algorithms except for dijkstra

changelog

87ef2ea

more 'official' and 'permissive' fixes

434e68f

oops, edgelabel was one byte bigger all of a sudden

cf1ed9f

nilsnolde force-pushed the nn-hgv-no-penalty branch from 871c90f to cf1ed9f Compare March 25, 2024 15:16

nilsnolde added 2 commits March 25, 2024 16:17

revert ped stuff

84186dd

bitwise or car & truck access when that penalty is on

41bb7d9

nilsnolde commented Mar 25, 2024

View reviewed changes

Merge branch 'master' into nn-hgv-no-penalty

c5b037a

kevinkreiser reviewed Apr 2, 2024

View reviewed changes

kevinkreiser approved these changes Apr 2, 2024

View reviewed changes

nilsnolde merged commit fdf1204 into master Apr 2, 2024
9 checks passed

nilsnolde deleted the nn-hgv-no-penalty branch April 2, 2024 09:34

kochis mentioned this pull request Apr 19, 2024

Debugging reasons for truck re-route #4697

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

truck penalty for hgv=no edges #4650

truck penalty for hgv=no edges #4650

nilsnolde commented Mar 22, 2024 •

edited

nilsnolde Mar 22, 2024

kevinkreiser Apr 2, 2024

nilsnolde Mar 22, 2024

kevinkreiser Apr 2, 2024

nilsnolde Mar 22, 2024

kevinkreiser Apr 2, 2024

nilsnolde Mar 22, 2024

nilsnolde Mar 22, 2024

nilsnolde Mar 22, 2024

nilsnolde Mar 22, 2024

nilsnolde Mar 25, 2024

nilsnolde commented Mar 25, 2024

kevinkreiser Apr 2, 2024

nilsnolde Apr 2, 2024

kevinkreiser Apr 2, 2024

kevinkreiser Apr 2, 2024

kevinkreiser Apr 2, 2024

nilsnolde commented Apr 2, 2024

truck penalty for hgv=no edges #4650

truck penalty for hgv=no edges #4650

Conversation

nilsnolde commented Mar 22, 2024 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nilsnolde commented Mar 25, 2024

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

nilsnolde commented Apr 2, 2024

nilsnolde commented Mar 22, 2024 •

edited