Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

truck penalty for hgv=no edges #4650

Merged
merged 9 commits into from Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -106,6 +106,7 @@
* ADDED: return isotile grid as geotiff [#4594](https://github.com/valhalla/valhalla/pull/4594)
* ADDED: `ignore_non_vehicular_restrictions` parameter for truck costing [#4606](https://github.com/valhalla/valhalla/pull/4606)
* UPDATED: tz database to 2024a [#4643](https://github.com/valhalla/valhalla/pull/4643)
* ADDED: `hgv_no_penalty` costing option to allow penalized truck access to `hgv=no` edges [#4650](https://github.com/valhalla/valhalla/pull/4650)

## Release Date: 2023-05-11 Valhalla 3.4.0
* **Removed**
Expand Down
1 change: 1 addition & 0 deletions docs/docs/api/turn-by-turn/api-reference.md
Expand Up @@ -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. |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open for a different name:)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


##### Bicycle costing options
The default bicycle costing is tuned toward road bicycles with a slight preference for using [cycleways](http://wiki.openstreetmap.org/wiki/Key:cycleway) or roads with bicycle lanes. Bicycle routes use regular roads where needed or where no direct bicycle lane options exist, but avoid roads without bicycle access. The costing model recognizes several factors unique to bicycle travel and offers several options for tuning bicycle routes. Several factors unique to travel by bicycle influence the resulting route.
Expand Down
22 changes: 11 additions & 11 deletions lua/graph.lua
Expand Up @@ -91,7 +91,7 @@ access = {
["forestry"] = "false",
["destination"] = "true",
["customers"] = "true",
["official"] = "false",
["official"] = "true",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU official and permissive are signifying allowed access

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

["public"] = "true",
["restricted"] = "true",
["allowed"] = "true",
Expand Down Expand Up @@ -155,7 +155,7 @@ motor_vehicle = {
["forestry"] = "false",
["destination"] = "true",
["customers"] = "true",
["official"] = "false",
["official"] = "true",
["public"] = "true",
["restricted"] = "true",
["allowed"] = "true",
Expand Down Expand Up @@ -232,7 +232,7 @@ bus = {
["restricted"] = "true",
["destination"] = "true",
["delivery"] = "false",
["official"] = "false",
["official"] = "true",
["permit"] = "true"
}

Expand All @@ -245,7 +245,7 @@ taxi = {
["restricted"] = "true",
["destination"] = "true",
["delivery"] = "false",
["official"] = "false",
["official"] = "true",
["permit"] = "true"
}

Expand All @@ -270,10 +270,10 @@ truck = {
["agricultural"] = "false",
["private"] = "true",
["discouraged"] = "false",
["permissive"] = "false",
["permissive"] = "true",
["unsuitable"] = "false",
["agricultural;forestry"] = "false",
["official"] = "false",
["official"] = "true",
["forestry"] = "false",
["destination;delivery"] = "true",
["permit"] = "true",
Expand Down Expand Up @@ -537,7 +537,7 @@ motor_cycle_node = {
["forestry"] = 0,
["destination"] = 1024,
["customers"] = 1024,
["official"] = 0,
["official"] = 1024,
["public"] = 1024,
["restricted"] = 1024,
["allowed"] = 1024,
Expand All @@ -553,7 +553,7 @@ bus_node = {
["restricted"] = 64,
["destination"] = 64,
["delivery"] = 0,
["official"] = 0,
["official"] = 64,
["permit"] = 64
}

Expand All @@ -566,7 +566,7 @@ taxi_node = {
["restricted"] = 32,
["destination"] = 32,
["delivery"] = 0,
["official"] = 0,
["official"] = 32,
["permit"] = 32
}

Expand All @@ -580,10 +580,10 @@ truck_node = {
["agricultural"] = 0,
["private"] = 8,
["discouraged"] = 0,
["permissive"] = 0,
["permissive"] = 8,
["unsuitable"] = 0,
["agricultural;forestry"] = 0,
["official"] = 0,
["official"] = 8,
["forestry"] = 0,
["destination;delivery"] = 8,
["permit"] = 8,
Expand Down
3 changes: 3 additions & 0 deletions proto/options.proto
Expand Up @@ -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 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has a default of kMaxPenalty

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

float hgv_no_access_penalty = 85;
}
}

oneof has_options {
Expand Down
3 changes: 2 additions & 1 deletion src/mjolnir/pbfgraphparser.cc
Expand Up @@ -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");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"restrictions" is a bit too generic IMO

LOG_INFO("Finished with " + std::to_string(osmdata.lane_connectivity_map.size()) +
" lane connections");

Expand Down
2 changes: 1 addition & 1 deletion src/mjolnir/restrictionbuilder.cc
Expand Up @@ -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));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

for (size_t i = 0; i < threads.size(); ++i) {
threads[i].reset(new std::thread(build, std::cref(complex_from_restrictions_file),
std::cref(complex_to_restrictions_file),
Expand Down
38 changes: 29 additions & 9 deletions src/sif/truckcost.cc
Expand Up @@ -88,16 +88,17 @@ constexpr float kSurfaceFactor[] = {
};

// Valid ranges and defaults
constexpr ranged_default_t<float> kLowClassPenaltyRange{0, kDefaultLowClassPenalty, kMaxPenalty};
constexpr ranged_default_t<float> kTruckWeightRange{0, kDefaultTruckWeight, 100.0f};
constexpr ranged_default_t<float> kTruckAxleLoadRange{0, kDefaultTruckAxleLoad, 40.0f};
constexpr ranged_default_t<float> kTruckHeightRange{0, kDefaultTruckHeight, 10.0f};
constexpr ranged_default_t<float> kTruckWidthRange{0, kDefaultTruckWidth, 10.0f};
constexpr ranged_default_t<float> kTruckLengthRange{0, kDefaultTruckLength, 50.0f};
constexpr ranged_default_t<float> kUseTollsRange{0, kDefaultUseTolls, 1.0f};
constexpr ranged_default_t<float> kLowClassPenaltyRange{0.f, kDefaultLowClassPenalty, kMaxPenalty};
constexpr ranged_default_t<float> kTruckWeightRange{0.f, kDefaultTruckWeight, 100.0f};
constexpr ranged_default_t<float> kTruckAxleLoadRange{0.f, kDefaultTruckAxleLoad, 40.0f};
constexpr ranged_default_t<float> kTruckHeightRange{0.f, kDefaultTruckHeight, 10.0f};
constexpr ranged_default_t<float> kTruckWidthRange{0.f, kDefaultTruckWidth, 10.0f};
constexpr ranged_default_t<float> kTruckLengthRange{0.f, kDefaultTruckLength, 50.0f};
constexpr ranged_default_t<float> kUseTollsRange{0.f, kDefaultUseTolls, 1.0f};
constexpr ranged_default_t<uint32_t> kAxleCountRange{2, kDefaultAxleCount, 20};
constexpr ranged_default_t<float> kUseHighwaysRange{0, kDefaultUseHighways, 1.0f};
constexpr ranged_default_t<float> kTopSpeedRange{10, kMaxAssumedTruckSpeed, kMaxSpeedKph};
constexpr ranged_default_t<float> kUseHighwaysRange{0.f, kDefaultUseHighways, 1.0f};
constexpr ranged_default_t<float> kTopSpeedRange{10.f, kMaxAssumedTruckSpeed, kMaxSpeedKph};
constexpr ranged_default_t<float> kHGVNoAccessRange{0.f, kMaxPenalty, kMaxPenalty};

BaseCostingOptionsConfig GetBaseCostOptsConfig() {
BaseCostingOptionsConfig cfg{};
Expand Down Expand Up @@ -311,6 +312,9 @@ class TruckCost : public DynamicCost {

// Density factor used in edge transition costing
std::vector<float> trans_density_factor_;

// determine if we should allow hgv=no edges and penalize them instead
float no_hgv_access_penalty_;
};

// Constructor
Expand Down Expand Up @@ -368,6 +372,12 @@ TruckCost::TruckCost(const Costing& costing)
for (uint32_t d = 0; d < 16; d++) {
density_factor_[d] = 0.85f + (d * 0.025f);
}

// 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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// set the access mask to both car & truck if that penalty is active
access_mask_ = no_hgv_access_penalty_active ? (kAutoAccess | kTruckAccess) : kTruckAccess;
}

// Destructor
Expand Down Expand Up @@ -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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool so we only penalize on entering it 👍


// Transition time = turncost * stopimpact * densityfactor
if (edge->stopimpact(idx) > 0 && !shortest_) {
float turn_cost;
Expand Down Expand Up @@ -624,6 +638,10 @@ Cost TruckCost::TransitionCostReverse(const uint32_t idx,
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->forwardaccess() & kTruckAccess) && !(edge->forwardaccess() & kTruckAccess));

// Transition time = turncost * stopimpact * densityfactor
if (edge->stopimpact(idx) > 0 && !shortest_) {
float turn_cost;
Expand Down Expand Up @@ -713,6 +731,8 @@ void ParseTruckCostOptions(const rapidjson::Document& doc,
JSON_PBF_RANGED_DEFAULT(co, kUseHighwaysRange, json, "/use_highways", use_highways);
JSON_PBF_RANGED_DEFAULT_V2(co, kAxleCountRange, json, "/axle_count", axle_count);
JSON_PBF_RANGED_DEFAULT(co, kTopSpeedRange, json, "/top_speed", top_speed);
JSON_PBF_RANGED_DEFAULT(co, kHGVNoAccessRange, json, "/hgv_no_access_penalty",
hgv_no_access_penalty);
}

cost_ptr_t CreateTruckCost(const Costing& costing_options) {
Expand Down
12 changes: 8 additions & 4 deletions src/thor/bidirectional_astar.cc
Expand Up @@ -329,7 +329,8 @@ inline bool BidirectionalAStar::ExpandInner(baldr::GraphReader& graphreader,
costing_->TurnType(pred.opp_local_idx(), nodeinfo, meta.edge),
restriction_idx, 0,
meta.edge->destonly() ||
(costing_->is_hgv() && meta.edge->destonly_hgv()));
(costing_->is_hgv() && meta.edge->destonly_hgv()),
meta.edge->forwardaccess() & kTruckAccess);
adjacencylist_forward_.add(idx);
} else {
idx = edgelabels_reverse_.size();
Expand All @@ -346,7 +347,8 @@ inline bool BidirectionalAStar::ExpandInner(baldr::GraphReader& graphreader,
opp_pred_edge),
restriction_idx, 0,
opp_edge->destonly() ||
(costing_->is_hgv() && opp_edge->destonly_hgv()));
(costing_->is_hgv() && opp_edge->destonly_hgv()),
opp_edge->forwardaccess() & kTruckAccess);
adjacencylist_reverse_.add(idx);
}

Expand Down Expand Up @@ -1017,7 +1019,8 @@ void BidirectionalAStar::SetOrigin(GraphReader& graphreader,
static_cast<bool>(flow_sources & kDefaultFlowMask),
sif::InternalTurn::kNoTurn, 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
adjacencylist_forward_.add(idx);

// setting this edge as reached
Expand Down Expand Up @@ -1113,7 +1116,8 @@ void BidirectionalAStar::SetDestination(GraphReader& graphreader,
static_cast<bool>(flow_sources & kDefaultFlowMask),
sif::InternalTurn::kNoTurn, kInvalidRestriction,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
adjacencylist_reverse_.add(idx);

// setting this edge as reached, sending the opposing because this is the reverse tree
Expand Down
12 changes: 8 additions & 4 deletions src/thor/costmatrix.cc
Expand Up @@ -527,7 +527,8 @@ bool CostMatrix::ExpandInner(baldr::GraphReader& graphreader,
costing_->TurnType(pred.opp_local_idx(), nodeinfo, meta.edge),
restriction_idx, 0,
meta.edge->destonly() ||
(costing_->is_hgv() && meta.edge->destonly_hgv()));
(costing_->is_hgv() && meta.edge->destonly_hgv()),
meta.edge->forwardaccess() & kTruckAccess);
} else {
edgelabels.emplace_back(pred_idx, meta.edge_id, opp_edge_id, meta.edge, newcost, mode_, tc,
pred_dist, not_thru_pruning,
Expand All @@ -536,7 +537,8 @@ bool CostMatrix::ExpandInner(baldr::GraphReader& graphreader,
costing_->TurnType(meta.edge->localedgeidx(), nodeinfo, opp_edge,
opp_pred_edge),
restriction_idx, 0,
opp_edge->destonly() || (costing_->is_hgv() && opp_edge->destonly_hgv()));
opp_edge->destonly() || (costing_->is_hgv() && opp_edge->destonly_hgv()),
opp_edge->forwardaccess() & kTruckAccess);
}
adj.add(idx);
// mark the edge as settled for the connection check
Expand Down Expand Up @@ -1030,7 +1032,8 @@ void CostMatrix::SetSources(GraphReader& graphreader,
InternalTurn::kNoTurn, kInvalidRestriction,
static_cast<uint8_t>(costing_->Allowed(directededge, tile)),
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
edge_label.set_not_thru(false);

// Add EdgeLabel to the adjacency list (but do not set its status).
Expand Down Expand Up @@ -1114,7 +1117,8 @@ void CostMatrix::SetTargets(baldr::GraphReader& graphreader,
InternalTurn::kNoTurn, kInvalidRestriction,
static_cast<uint8_t>(costing_->Allowed(opp_dir_edge, opp_tile)),
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
edge_label.set_not_thru(false);

// Add EdgeLabel to the adjacency list (but do not set its status).
Expand Down
12 changes: 8 additions & 4 deletions src/thor/dijkstras.cc
Expand Up @@ -254,7 +254,8 @@ void Dijkstras::ExpandInner(baldr::GraphReader& graphreader,
costing_->TurnType(pred.opp_local_idx(), nodeinfo, directededge),
restriction_idx, pred.path_id(),
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);

} else {
bdedgelabels_.emplace_back(pred_idx, edgeid, oppedgeid, directededge, newcost, mode_,
Expand All @@ -265,7 +266,8 @@ void Dijkstras::ExpandInner(baldr::GraphReader& graphreader,
opp_pred_edge),
restriction_idx, pred.path_id(),
opp_edge->destonly() ||
(costing_->is_hgv() && opp_edge->destonly_hgv()));
(costing_->is_hgv() && opp_edge->destonly_hgv()),
opp_edge->forwardaccess() & kTruckAccess);
}
adjacencylist_.add(idx);
}
Expand Down Expand Up @@ -823,7 +825,8 @@ void Dijkstras::SetOriginLocations(GraphReader& graphreader,
static_cast<bool>(flow_sources & kDefaultFlowMask),
InternalTurn::kNoTurn, kInvalidRestriction, multipath_ ? path_id : 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
// Set the origin flag
bdedgelabels_.back().set_origin();

Expand Down Expand Up @@ -914,7 +917,8 @@ void Dijkstras::SetDestinationLocations(
static_cast<bool>(flow_sources & kDefaultFlowMask),
InternalTurn::kNoTurn, restriction_idx, multipath_ ? path_id : 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
adjacencylist_.add(idx);
edgestatus_.Set(opp_edge_id, EdgeSet::kTemporary, idx, opp_tile, multipath_ ? path_id : 0);
}
Expand Down
12 changes: 8 additions & 4 deletions src/thor/timedistancematrix.cc
Expand Up @@ -156,7 +156,8 @@ void TimeDistanceMatrix::Expand(GraphReader& graphreader,
0 != (flow_sources & kDefaultFlowMask),
costing_->TurnType(pred.opp_local_idx(), nodeinfo, directededge), 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
} else {
edgelabels_.emplace_back(pred_idx, edgeid, directededge, newcost, newcost.cost, mode_,
path_distance, restriction_idx,
Expand All @@ -166,7 +167,8 @@ void TimeDistanceMatrix::Expand(GraphReader& graphreader,
opp_pred_edge),
0,
opp_edge->destonly() ||
(costing_->is_hgv() && opp_edge->destonly_hgv()));
(costing_->is_hgv() && opp_edge->destonly_hgv()),
opp_edge->forwardaccess() & kTruckAccess);
}

*es = {EdgeSet::kTemporary, idx};
Expand Down Expand Up @@ -374,14 +376,16 @@ void TimeDistanceMatrix::SetOrigin(GraphReader& graphreader,
static_cast<bool>(flow_sources & kDefaultFlowMask),
InternalTurn::kNoTurn, 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
} else {
edgelabels_.emplace_back(kInvalidLabel, opp_edge_id, opp_dir_edge, cost, cost.cost, mode_, dist,
baldr::kInvalidRestriction, !costing_->IsClosed(directededge, tile),
static_cast<bool>(flow_sources & kDefaultFlowMask),
InternalTurn::kNoTurn, 0,
directededge->destonly() ||
(costing_->is_hgv() && directededge->destonly_hgv()));
(costing_->is_hgv() && directededge->destonly_hgv()),
directededge->forwardaccess() & kTruckAccess);
}
edgelabels_.back().set_origin();
adjacencylist_.add(edgelabels_.size() - 1);
Expand Down