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.

Sign up for GitHub

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

Penalize private gates #3144

Merged
merged 4 commits into from
Jun 22, 2021
Merged

Penalize private gates #3144

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35545d0
penalize private gates
CuriOusQuOkka Jun 1, 2021
527bb52
penalize private nodes separately
CuriOusQuOkka Jun 10, 2021
a19497c
use one parameter and do not tune bicycle
CuriOusQuOkka Jun 17, 2021
ef8a0a7
update doc
CuriOusQuOkka Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
use one parameter and do not tune bicycle
  • Loading branch information
@CuriOusQuOkka
CuriOusQuOkka committed Jun 21, 2021
commit a19497c3c9d61b28f3cdc62593e90d3cf080ffce
3 changes: 1 addition & 2 deletions proto/options.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ message CostingOptions {
optional float use_living_streets = 68;
optional float service_factor = 69;
optional float closure_factor = 70;
optional float private_access_cost = 71;
optional float private_access_penalty = 72;
optional float private_access_penalty = 71;

// these are not specified directly by the user but they get filled in as the request is parsed and fulfilled
optional Costing costing = 90;
Expand Down
10 changes: 4 additions & 6 deletions src/sif/bicyclecost.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ namespace sif {
namespace {

// Base transition costs
constexpr float kDefaultAlleyPenalty = 60.0f; // Seconds
constexpr float kDefaultGatePenalty = 600.0f; // Seconds
constexpr float kDefaultPrivateAccessPenalty = 750.0f; // Seconds
constexpr float kDefaultBssCost = 120.0f; // Seconds
constexpr float kDefaultBssPenalty = 0.0f; // Seconds
constexpr float kDefaultAlleyPenalty = 60.0f; // Seconds
constexpr float kDefaultGatePenalty = 600.0f; // Seconds
constexpr float kDefaultBssCost = 120.0f; // Seconds
constexpr float kDefaultBssPenalty = 0.0f; // Seconds

// Other options
constexpr float kDefaultUseRoad = 0.25f; // Factor between 0 and 1
Expand Down Expand Up @@ -197,7 +196,6 @@ BaseCostingOptionsConfig GetBaseCostOptsConfig() {
// override defaults
cfg.alley_penalty_.def = kDefaultAlleyPenalty;
cfg.gate_penalty_.def = kDefaultGatePenalty;
cfg.private_access_penalty_.def = kDefaultPrivateAccessPenalty;
cfg.disable_toll_booth_ = true;
cfg.disable_rail_ferry_ = true;
cfg.use_living_streets_.def = kDefaultUseLivingStreets;
Expand Down
7 changes: 0 additions & 7 deletions src/sif/dynamiccost.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ constexpr float kDefaultManeuverPenalty = 5.0f; // Seconds
constexpr float kDefaultAlleyPenalty = 5.0f; // Seconds
constexpr float kDefaultGateCost = 30.0f; // Seconds
constexpr float kDefaultGatePenalty = 300.0f; // Seconds
constexpr float kDefaultPrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultPrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultTollBoothCost = 15.0f; // Seconds
constexpr float kDefaultTollBoothPenalty = 0.0f; // Seconds
Expand Down Expand Up @@ -118,7 +117,6 @@ BaseCostingOptionsConfig::BaseCostingOptionsConfig()
alley_penalty_{0.f, kDefaultAlleyPenalty, kMaxPenalty},
gate_cost_{0.f, kDefaultGateCost, kMaxPenalty}, gate_penalty_{0.f, kDefaultGatePenalty,
kMaxPenalty},
private_access_cost_{0.f, kDefaultPrivateAccessCost, kMaxPenalty},
private_access_penalty_{0.f, kDefaultPrivateAccessPenalty, kMaxPenalty},
country_crossing_cost_{0.f, kDefaultCountryCrossingCost, kMaxPenalty},
country_crossing_penalty_{0.f, kDefaultCountryCrossingPenalty, kMaxPenalty},
Expand Down Expand Up @@ -384,10 +382,6 @@ void ParseBaseCostOptions(const rapidjson::Value& value,
pbf_costing_options->set_gate_penalty(base_cfg.gate_penalty_(
rapidjson::get<float>(value, "/gate_penalty", base_cfg.gate_penalty_.def)));

// private_access_cost
pbf_costing_options->set_private_access_cost(base_cfg.private_access_cost_(
rapidjson::get<float>(value, "/private_access_cost", base_cfg.private_access_cost_.def)));

// private_access_penalty
pbf_costing_options->set_private_access_penalty(base_cfg.private_access_penalty_(
rapidjson::get<float>(value, "/private_access_penalty", base_cfg.private_access_penalty_.def)));
Expand Down Expand Up @@ -459,7 +453,6 @@ void SetDefaultBaseCostOptions(CostingOptions* pbf_costing_options,
pbf_costing_options->set_alley_penalty(shared_opts.alley_penalty_.def);
pbf_costing_options->set_gate_cost(shared_opts.gate_cost_.def);
pbf_costing_options->set_gate_penalty(shared_opts.gate_penalty_.def);
pbf_costing_options->set_private_access_cost(shared_opts.private_access_cost_.def);
pbf_costing_options->set_private_access_penalty(shared_opts.private_access_penalty_.def);
pbf_costing_options->set_country_crossing_cost(shared_opts.country_crossing_cost_.def);
pbf_costing_options->set_country_crossing_penalty(shared_opts.country_crossing_penalty_.def);
Expand Down
4 changes: 1 addition & 3 deletions src/sif/pedestriancost.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,7 @@ class PedestrianCost : public DynamicCost {
sif::Cost c;
c += country_crossing_cost_ * (node->type() == baldr::NodeType::kBorderControl);
c += gate_cost_ * (node->type() == baldr::NodeType::kGate) * (!node->tagged_access());
c += private_access_cost_ *
(node->type() == baldr::NodeType::kGate || node->type() == baldr::NodeType::kBollard) *
node->private_access();
c += private_access_cost_ * (node->type() == baldr::NodeType::kGate) * node->private_access();
c += bike_share_cost_ * (node->type() == baldr::NodeType::kBikeShare);
c += ferry_transition_cost_ *
(edge->use() == baldr::Use::kFerry && pred->use() != baldr::Use::kFerry);
Expand Down
2 changes: 1 addition & 1 deletion taginfo.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
"node",
"way"
],
"description": "Sets access flag to true; however, if access = private and (emergency = yes or service = emergency_access) then access flag is false. In case a gate or bollard has access = private the node will be avoided if there is a path that does not include it"
"description": "Sets access flag to true; however, if access = private and (emergency = yes or service = emergency_access) then access flag is false."
},
{
"key": "access",
Expand Down
4 changes: 2 additions & 2 deletions test/isochrone.cc
View file
Open in desktop

Large diffs are not rendered by default.

77 changes: 1 addition & 76 deletions test/parse_request.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ constexpr float kDefaultAuto_DestinationOnlyPenalty = 600.0f; // Seconds
constexpr float kDefaultAuto_AlleyPenalty = 5.0f; // Seconds
constexpr float kDefaultAuto_GateCost = 30.0f; // Seconds
constexpr float kDefaultAuto_GatePenalty = 300.0f; // Seconds
constexpr float kDefaultAuto_PrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultAuto_PrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultAuto_TollBoothCost = 15.0f; // Seconds
constexpr float kDefaultAuto_TollBoothPenalty = 0.0f; // Seconds
Expand All @@ -42,7 +41,6 @@ constexpr float kDefaultMotorScooter_ManeuverPenalty = 5.0f; // Seconds
constexpr float kDefaultMotorScooter_AlleyPenalty = 5.0f; // Seconds
constexpr float kDefaultMotorScooter_GateCost = 30.0f; // Seconds
constexpr float kDefaultMotorScooter_GatePenalty = 300.0f; // Seconds
constexpr float kDefaultMotorScooter_PrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultMotorScooter_PrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultMotorScooter_FerryCost = 300.0f; // Seconds
constexpr float kDefaultMotorScooter_CountryCrossingCost = 600.0f; // Seconds
Expand All @@ -61,7 +59,6 @@ constexpr float kDefaultMotorcycle_ManeuverPenalty = 5.0f; // Seconds
constexpr float kDefaultMotorcycle_AlleyPenalty = 5.0f; // Seconds
constexpr float kDefaultMotorcycle_GateCost = 30.0f; // Seconds
constexpr float kDefaultMotorcycle_GatePenalty = 300.0f; // Seconds
constexpr float kDefaultMotorcycle_PrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultMotorcycle_PrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultMotorcycle_TollBoothCost = 15.0f; // Seconds
constexpr float kDefaultMotorcycle_TollBoothPenalty = 0.0f; // Seconds
Expand Down Expand Up @@ -106,12 +103,10 @@ constexpr float kDefaultPedestrian_ServiceFactor = 1.f; // Posi

// Bicycle defaults
constexpr float kDefaultBicycle_ManeuverPenalty = 5.0f; // Seconds
constexpr float kDefaultBicycle_DrivewayPenalty = 300.0f; // Seconds
constexpr float kDefaultBicycle_AlleyPenalty = 60.0f; // Seconds
constexpr float kDefaultBicycle_GateCost = 30.0f; // Seconds
constexpr float kDefaultBicycle_GatePenalty = 600.0f; // Seconds
constexpr float kDefaultBicycle_PrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultBicycle_PrivateAccessPenalty = 750.0f; // Seconds
constexpr float kDefaultBicycle_PrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultBicycle_FerryCost = 300.0f; // Seconds
constexpr float kDefaultBicycle_CountryCrossingCost = 600.0f; // Seconds
constexpr float kDefaultBicycle_CountryCrossingPenalty = 0.0f; // Seconds
Expand All @@ -135,7 +130,6 @@ constexpr float kDefaultTruck_DestinationOnlyPenalty = 600.0f; // Seconds
constexpr float kDefaultTruck_AlleyPenalty = 5.0f; // Seconds
constexpr float kDefaultTruck_GateCost = 30.0f; // Seconds
constexpr float kDefaultTruck_GatePenalty = 300.0f; // Seconds
constexpr float kDefaultTruck_PrivateAccessCost = 5.0f; // Seconds
constexpr float kDefaultTruck_PrivateAccessPenalty = 450.0f; // Seconds
constexpr float kDefaultTruck_TollBoothCost = 15.0f; // Seconds
constexpr float kDefaultTruck_TollBoothPenalty = 0.0f; // Seconds
Expand Down Expand Up @@ -485,8 +479,6 @@ void test_default_base_auto_cost_options(const Costing costing, const Options::A
request.options().costing_options(static_cast<int>(costing)).gate_cost());
validate("gate_penalty", kDefaultAuto_GatePenalty,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
validate("private_access_cost", kDefaultAuto_PrivateAccessCost,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
validate("private_access_penalty", kDefaultAuto_PrivateAccessPenalty,
request.options().costing_options(static_cast<int>(costing)).private_access_penalty());
validate("toll_booth_cost", kDefaultAuto_TollBoothCost,
Expand Down Expand Up @@ -533,8 +525,6 @@ void test_default_motor_scooter_cost_options(const Costing costing, const Option
request.options().costing_options(static_cast<int>(costing)).gate_cost());
validate("gate_penalty", kDefaultMotorScooter_GatePenalty,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
validate("private_access_cost", kDefaultMotorScooter_PrivateAccessCost,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
validate("private_access_penalty", kDefaultMotorScooter_PrivateAccessPenalty,
request.options().costing_options(static_cast<int>(costing)).private_access_penalty());
validate("alley_penalty", kDefaultMotorScooter_AlleyPenalty,
Expand Down Expand Up @@ -577,8 +567,6 @@ void test_default_motorcycle_cost_options(const Costing costing, const Options::
request.options().costing_options(static_cast<int>(costing)).gate_cost());
validate("gate_penalty", kDefaultMotorcycle_GatePenalty,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
validate("private_access_cost", kDefaultMotorcycle_PrivateAccessCost,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
validate("private_access_penalty", kDefaultMotorcycle_PrivateAccessPenalty,
request.options().costing_options(static_cast<int>(costing)).private_access_penalty());
validate("alley_penalty", kDefaultMotorcycle_AlleyPenalty,
Expand Down Expand Up @@ -679,8 +667,6 @@ void test_default_bicycle_cost_options(const Costing costing, const Options::Act
request.options().costing_options(static_cast<int>(costing)).gate_cost());
validate("gate_penalty", kDefaultBicycle_GatePenalty,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
validate("private_access_cost", kDefaultBicycle_PrivateAccessCost,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
validate("private_access_penalty", kDefaultBicycle_PrivateAccessPenalty,
request.options().costing_options(static_cast<int>(costing)).private_access_penalty());
validate("country_crossing_cost", kDefaultBicycle_CountryCrossingCost,
Expand Down Expand Up @@ -724,8 +710,6 @@ void test_default_truck_cost_options(const Costing costing, const Options::Actio
request.options().costing_options(static_cast<int>(costing)).gate_cost());
validate("gate_penalty", kDefaultTruck_GatePenalty,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
validate("private_access_cost", kDefaultTruck_PrivateAccessCost,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
validate("private_access_penalty", kDefaultTruck_PrivateAccessPenalty,
request.options().costing_options(static_cast<int>(costing)).private_access_penalty());
validate("toll_booth_cost", kDefaultTruck_TollBoothCost,
Expand Down Expand Up @@ -877,22 +861,6 @@ void test_gate_penalty_parsing(const Costing costing,
request.options().costing_options(static_cast<int>(costing)).gate_penalty());
}

void test_private_access_cost_parsing(const Costing costing,
const float specified_value,
const float expected_value,
const Options::Action action = Options::route) {
// Create the costing string
auto costing_str = get_costing_str(costing);
const std::string grandparent_key = "costing_options";
const std::string& parent_key = costing_str;
const std::string key = "private_access_cost";

Api request =
get_request(get_request_str(grandparent_key, parent_key, key, specified_value), action);
validate(key, expected_value,
request.options().costing_options(static_cast<int>(costing)).private_access_cost());
}

void test_private_access_penalty_parsing(const Costing costing,
const float specified_value,
const float expected_value,
Expand Down Expand Up @@ -2050,49 +2018,6 @@ TEST(ParseRequest, test_gate_penalty) {
test_gate_penalty_parsing(costing, 50000.f, default_value);
}

TEST(ParseRequest, test_private_access_cost) {
float default_value = kDefaultAuto_PrivateAccessCost;
for (auto costing : get_base_auto_costing_list()) {
test_private_access_cost_parsing(costing, default_value, default_value);
test_private_access_cost_parsing(costing, 2.f, 2.f);
test_private_access_cost_parsing(costing, 60.f, 60.f);
test_private_access_cost_parsing(costing, -2.f, default_value);
test_private_access_cost_parsing(costing, 500000.f, default_value);
}

Costing costing = Costing::motor_scooter;
default_value = kDefaultMotorScooter_PrivateAccessCost;
test_private_access_cost_parsing(costing, default_value, default_value);
test_private_access_cost_parsing(costing, 2.f, 2.f);
test_private_access_cost_parsing(costing, 60.f, 60.f);
test_private_access_cost_parsing(costing, -2.f, default_value);
test_private_access_cost_parsing(costing, 500000.f, default_value);

costing = Costing::motorcycle;
default_value = kDefaultMotorcycle_PrivateAccessCost;
test_private_access_cost_parsing(costing, default_value, default_value);
test_private_access_cost_parsing(costing, 2.f, 2.f);
test_private_access_cost_parsing(costing, 60.f, 60.f);
test_private_access_cost_parsing(costing, -2.f, default_value);
test_private_access_cost_parsing(costing, 500000.f, default_value);

costing = Costing::bicycle;
default_value = kDefaultBicycle_PrivateAccessCost;
test_private_access_cost_parsing(costing, default_value, default_value);
test_private_access_cost_parsing(costing, 15.f, 15.f);
test_private_access_cost_parsing(costing, 60.f, 60.f);
test_private_access_cost_parsing(costing, -2.f, default_value);
test_private_access_cost_parsing(costing, 50000.f, default_value);

costing = Costing::truck;
default_value = kDefaultTruck_PrivateAccessCost;
test_private_access_cost_parsing(costing, default_value, default_value);
test_private_access_cost_parsing(costing, 15.f, 15.f);
test_private_access_cost_parsing(costing, 60.f, 60.f);
test_private_access_cost_parsing(costing, -2.f, default_value);
test_private_access_cost_parsing(costing, 50000.f, default_value);
}

TEST(ParseRequest, test_private_access_penalty) {
float default_value = kDefaultAuto_PrivateAccessPenalty;
for (auto costing : get_base_auto_costing_list()) {
Expand Down
10 changes: 3 additions & 7 deletions valhalla/sif/dynamiccost.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,9 +904,8 @@ class DynamicCost {
costing_options.country_crossing_cost()};
gate_cost_ = {costing_options.gate_cost() + costing_options.gate_penalty(),
costing_options.gate_cost()};
private_access_cost_ = {costing_options.private_access_cost() +
costing_options.private_access_penalty(),
costing_options.private_access_cost()};
private_access_cost_ = {costing_options.gate_cost() + costing_options.private_access_penalty(),
costing_options.gate_cost()};

bike_share_cost_ = {costing_options.bike_share_cost() + costing_options.bike_share_penalty(),
costing_options.bike_share_cost()};
Expand Down Expand Up @@ -997,9 +996,7 @@ class DynamicCost {
sif::Cost c;
c += country_crossing_cost_ * (node->type() == baldr::NodeType::kBorderControl);
c += gate_cost_ * (node->type() == baldr::NodeType::kGate) * (!node->tagged_access());
c += private_access_cost_ *
(node->type() == baldr::NodeType::kGate || node->type() == baldr::NodeType::kBollard) *
node->private_access();
c += private_access_cost_ * (node->type() == baldr::NodeType::kGate) * node->private_access();
c += bike_share_cost_ * (node->type() == baldr::NodeType::kBikeShare);
c += toll_booth_cost_ *
(node->type() == baldr::NodeType::kTollBooth || (edge->toll() && !pred->toll()));
Expand Down Expand Up @@ -1045,7 +1042,6 @@ struct BaseCostingOptionsConfig {
ranged_default_t<float> alley_penalty_;
ranged_default_t<float> gate_cost_;
ranged_default_t<float> gate_penalty_;
ranged_default_t<float> private_access_cost_;
ranged_default_t<float> private_access_penalty_;
ranged_default_t<float> country_crossing_cost_;
ranged_default_t<float> country_crossing_penalty_;
Expand Down