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
Changes from all commits
f6fd150
38e0604
4c49d20
87ef2ea
434e68f
cf1ed9f
84186dd
41bb7d9
c5b037a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,7 @@ access = { | |
["forestry"] = "false", | ||
["destination"] = "true", | ||
["customers"] = "true", | ||
["official"] = "false", | ||
["official"] = "true", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIU There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
@@ -155,7 +155,7 @@ motor_vehicle = { | |
["forestry"] = "false", | ||
["destination"] = "true", | ||
["customers"] = "true", | ||
["official"] = "false", | ||
["official"] = "true", | ||
["public"] = "true", | ||
["restricted"] = "true", | ||
["allowed"] = "true", | ||
|
@@ -232,7 +232,7 @@ bus = { | |
["restricted"] = "true", | ||
["destination"] = "true", | ||
["delivery"] = "false", | ||
["official"] = "false", | ||
["official"] = "true", | ||
["permit"] = "true" | ||
} | ||
|
||
|
@@ -245,7 +245,7 @@ taxi = { | |
["restricted"] = "true", | ||
["destination"] = "true", | ||
["delivery"] = "false", | ||
["official"] = "false", | ||
["official"] = "true", | ||
["permit"] = "true" | ||
} | ||
|
||
|
@@ -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", | ||
|
@@ -537,7 +537,7 @@ motor_cycle_node = { | |
["forestry"] = 0, | ||
["destination"] = 1024, | ||
["customers"] = 1024, | ||
["official"] = 0, | ||
["official"] = 1024, | ||
["public"] = 1024, | ||
["restricted"] = 1024, | ||
["allowed"] = 1024, | ||
|
@@ -553,7 +553,7 @@ bus_node = { | |
["restricted"] = 64, | ||
["destination"] = 64, | ||
["delivery"] = 0, | ||
["official"] = 0, | ||
["official"] = 64, | ||
["permit"] = 64 | ||
} | ||
|
||
|
@@ -566,7 +566,7 @@ taxi_node = { | |
["restricted"] = 32, | ||
["destination"] = 32, | ||
["delivery"] = 0, | ||
["official"] = 0, | ||
["official"] = 32, | ||
["permit"] = 32 | ||
} | ||
|
||
|
@@ -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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. has a default of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would have be cool to have the default be 0, not use |
||
float hgv_no_access_penalty = 85; | ||
} | ||
} | ||
|
||
oneof has_options { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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{}; | ||
|
@@ -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 | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
// set the access mask to both car & truck if that penalty is active | ||
access_mask_ = no_hgv_access_penalty_active ? (kAutoAccess | kTruckAccess) : kTruckAccess; | ||
} | ||
|
||
// Destructor | ||
|
@@ -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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
@@ -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; | ||
|
@@ -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) { | ||
|
There was a problem hiding this comment.
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:)
There was a problem hiding this comment.
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.