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

Making the turn function more flexible #4789

Merged
merged 37 commits into from
Jan 24, 2018
Merged
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
9792493
init commit
Jan 11, 2018
9450f13
prevent breaking API change
Jan 11, 2018
6775d86
set and store highway and access classification for the turn function
chaupow Jan 15, 2018
2a29c64
expose highway turn classification and access turn classification and…
chaupow Jan 15, 2018
329187c
expose whether connection road at turn is incoming or outgoing
chaupow Jan 15, 2018
2fd9442
small cleanup
chaupow Jan 17, 2018
76224ef
add lua tests for exposed information to turn function
chaupow Jan 17, 2018
096f86d
clang-format
chaupow Jan 17, 2018
9208fd7
remove mode in connected roads
chaupow Jan 17, 2018
69758c7
check lane numbers in tests
chaupow Jan 17, 2018
5767da9
update docs about attributes in process_turn
chaupow Jan 17, 2018
ac2846d
add turn_classification info to docs
chaupow Jan 17, 2018
3651f6b
small nitpicks and cleanup
chaupow Jan 17, 2018
0ad7169
review adjustments
chaupow Jan 18, 2018
09ff6ef
remove speed from node based edge and calculate from edge length and …
chaupow Jan 18, 2018
73c72ff
calculate speed by edge length and duration
chaupow Jan 18, 2018
62c089e
split roads on the left and roads on the right before running generat…
chaupow Jan 18, 2018
d1f3146
comment on u turns and empty roads_on_the_left
chaupow Jan 18, 2018
013d951
move profile to background
chaupow Jan 19, 2018
0611e55
move impl or setting info for connected edges
chaupow Jan 19, 2018
e51abc8
clang-format
chaupow Jan 19, 2018
3ee0d22
dirty commit! checking failing assertion
chaupow Jan 19, 2018
d339178
remove assertions for edge cases
chaupow Jan 19, 2018
430859c
less assertions
chaupow Jan 19, 2018
edc30b0
strange clang-format glitch
chaupow Jan 19, 2018
7898ccb
adding warning if uturn and intersection dont match
chaupow Jan 19, 2018
56754ea
handle u turns that do not turn into intersection[0]
chaupow Jan 19, 2018
939ac0f
attempt to make travis happy
chaupow Jan 22, 2018
989f633
try again
chaupow Jan 22, 2018
106874d
fix mismatch of arguments
chaupow Jan 22, 2018
98ca829
add changelog
chaupow Jan 19, 2018
271915e
add changelog
chaupow Jan 19, 2018
12629cd
fix changelog
chaupow Jan 23, 2018
dd61ce8
clang-format
chaupow Jan 23, 2018
a3de95c
fix typo and used std::transform instead of loop
chaupow Jan 24, 2018
9eb80ca
split OSM link generation in an accessible coordinate function
chaupow Jan 24, 2018
5550f76
guard transform when begin iterator is bigger than end iterator
chaupow Jan 24, 2018
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
small nitpicks and cleanup
chaupow committed Jan 23, 2018
commit 3651f6b3708117ca706bd9e6a80f7463dda94c34
3 changes: 2 additions & 1 deletion docs/profiles.md
Original file line number Diff line number Diff line change
@@ -296,7 +296,8 @@ end
function process_turn(profile, turn) {
if turn.source_highway_turn_classification == 2 and turn.target_highway_turn_classification == 2 then
turn.weight = 10
else if turn.source_highway_turn_classification == 1 and turn.target_highway_turn_classification == 1 then
end
if turn.source_highway_turn_classification == 1 and turn.target_highway_turn_classification == 1 then
turn.weight = 5
end
}
1 change: 1 addition & 0 deletions features/support/hooks.js
Original file line number Diff line number Diff line change
@@ -51,6 +51,7 @@ module.exports = function () {
.defer(rimraf, this.scenarioLogFile)
.awaitAll(callback);
// Question @review is this line nice to have here (commented, but ready to use and uncomment if needed?)
// i think this is super useful and would love to have it here
// console.log(" Writing logging output to " + this.scenarioLogFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

});

6 changes: 0 additions & 6 deletions include/extractor/extraction_turn.hpp
Original file line number Diff line number Diff line change
@@ -48,7 +48,6 @@ struct ExtractionTurn
int number_of_roads,
bool is_u_turn,
bool has_traffic_light,
// bool is_stop, bool is_give_way,
bool is_left_hand_driving,
bool source_restricted,
TravelMode source_mode,
@@ -93,16 +92,11 @@ struct ExtractionTurn
{
BOOST_ASSERT_MSG(!is_u_turn || roads_on_the_left.size() == 0,
"there cannot be roads on the left when there is a u turn");
BOOST_ASSERT_MSG(roads_on_the_right.size() + roads_on_the_left.size() + is_u_turn + 2 ==
number_of_roads,
"number of roads at intersection do not match");
}
const double angle;
const int number_of_roads;
const bool is_u_turn;
const bool has_traffic_light;
// const bool is_stop;
// const bool is_give_way;
const bool is_left_hand_driving;

// source info
4 changes: 2 additions & 2 deletions include/extractor/extraction_way.hpp
Original file line number Diff line number Diff line change
@@ -127,8 +127,8 @@ struct ExtractionWay
bool : 2;

// user classifications for turn penalties
std::uint8_t highway_turn_classification; // @CHAUTODO memory? limit to 3 bits? // 8
std::uint8_t access_turn_classification; // 3 are enough
std::uint8_t highway_turn_classification : 4;
std::uint8_t access_turn_classification : 4;
};
}
}
6 changes: 3 additions & 3 deletions include/extractor/node_based_edge.hpp
Original file line number Diff line number Diff line change
@@ -27,9 +27,9 @@ struct NodeBasedEdgeClassification
std::uint8_t startpoint : 1; // 1
std::uint8_t restricted : 1; // 1
guidance::RoadClassification road_classification; // 16 2
std::uint8_t highway_turn_classification; // @CHAUTODO memory? limit to 3 bits? // 8
std::uint8_t access_turn_classification; // 3 are enough // 8
std::uint8_t speed; // one bit could be saved here too I guess, so 7 // 8
std::uint8_t highway_turn_classification : 4; // 4
std::uint8_t access_turn_classification : 4; // 4
std::uint8_t speed; // 8
Copy link
Contributor

Choose a reason for hiding this comment

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

speed can be moved to NodeBasedEdge and grouped with weight and duration

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried calculating speed by edge length and duration. Many calculated speeds are within 10% of the actual speed. Some are not. Here is an extract of the differences running on Berlin:


calculated speed 50.000792 real speed 56
calculated speed 41.707704 real speed 48
calculated speed 41.707704 real speed 48
calculated speed 41.707704 real speed 48
calculated speed 30.748476 real speed 40
calculated speed 30.748476 real speed 40
calculated speed 32.505609 real speed 40
calculated speed 32.505609 real speed 40
calculated speed 30.754826 real speed 48
calculated speed 30.754826 real speed 48
calculated speed 30.754826 real speed 48
calculated speed 31.699496 real speed 40
calculated speed 31.699496 real speed 40
calculated speed 19.930782 real speed 25
calculated speed 19.930782 real speed 25
calculated speed 26.538870 real speed 55
calculated speed 26.538870 real speed 55
calculated speed 26.538870 real speed 55
calculated speed 19.930782 real speed 25

in favour of using less memory, @oxidase do you think this is preferable?

Copy link
Contributor

@oxidase oxidase Jan 18, 2018

Choose a reason for hiding this comment

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

@chaupow both speeds values have different meaning. For example, an NBG edge from https://www.openstreetmap.org/node/5339762636 to https://www.openstreetmap.org/node/1736058469 consists of two segments:

The speed value of the NBG edge is 40 kmh as the value of the first smaller segment.

The computed speed value is an average that also includes traffic light penalties.
In the example, the average speed is 26.44 kmh without traffic light penalty and 19.39 kmh with 2 seconds traffic light penalty.

I think average values are preferable over speed values of single segments.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think average values are preferable over speed values of single segments.

Agreed! Done thanks 👍


NodeBasedEdgeClassification();

2 changes: 1 addition & 1 deletion include/partition/multi_level_partition.hpp
Original file line number Diff line number Diff line change
@@ -148,7 +148,7 @@ template <storage::Ownership Ownership> class MultiLevelPartitionImpl final
auto offsets = MakeLevelOffsets(lidx_to_num_cells);
auto masks = MakeLevelMasks(offsets, num_level);
auto bits = MakeBitToLevel(offsets, num_level);
return std::make_unique<LevelData>(LevelData{num_level, offsets, masks, bits, {0}});
return std::make_unique<LevelData>(LevelData{num_level, offsets, masks, bits, {{0}}});
}

inline std::size_t LevelIDToIndex(LevelID l) const { return l - 1; }
7 changes: 7 additions & 0 deletions profiles/lib/way_handlers.lua
Original file line number Diff line number Diff line change
@@ -227,6 +227,13 @@ function WayHandlers.way_classification_for_turn(profile,way,result,data)
local highway = way:get_value_by_key("highway")
local access = way:get_value_by_key("access")

if profile.highway_turn_classification[highway] then
assert(profile.highway_turn_classification[highway] < 16, "highway_turn_classification must be smaller than 16")
Copy link
Contributor

Choose a reason for hiding this comment

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

Asserts can can be directly checked before assignments below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done 👍

end
if profile.access_turn_classification[highway] then
assert(profile.access_turn_classification[highway] < 16, "access_turn_classification must be smaller than 16")
end

if highway and profile.highway_turn_classification[highway] then
result.highway_turn_classification = profile.highway_turn_classification[highway]
end
11 changes: 9 additions & 2 deletions src/extractor/edge_based_graph_factory.cpp
Original file line number Diff line number Diff line change
@@ -599,14 +599,15 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(

// compute weight and duration penalties
auto is_traffic_light = m_traffic_lights.count(intersection_node);

std::vector<ExtractionTurnLeg> road_legs_on_the_right;
std::vector<ExtractionTurnLeg> road_legs_on_the_left;

auto turn_iter =
std::find_if(intersection.begin(), intersection.end(), [&turn](const auto &road) {
return road.eid == turn.eid;
});

// if the turn is a u turn, store all information in road_legs_on_the_right
Copy link
Contributor

Choose a reason for hiding this comment

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

Here can be an edge case for U-turns in left-driving countries.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think I should change this in left-driving countries?

I wonder whether it is important whether roads are on the left or right when you do a u turn and want to assign turn weights for u turns. Maybe it doesn't really matter here?

Copy link
Contributor

Choose a reason for hiding this comment

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

agree, it should not matter, but would good to mention u-turns in the description of ExtractionTurn:roads_on_the_right property

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 (turn_iter == intersection.begin())
{
for (auto connected_edge = intersection.begin() + 1;
@@ -627,6 +628,7 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
connected_edge->entry_allowed);
}
}
// otherwise split roads into the right or left side of the turn and store info
else
{
for (auto connected_edge = intersection.begin() + 1; connected_edge < turn_iter;
@@ -662,13 +664,16 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
connected_edge->entry_allowed);
}
}

ExtractionTurn extracted_turn(
// general info
turn.angle,
m_node_based_graph.GetOutDegree(intersection_node),
intersection.size(),
turn.instruction.IsUTurn(),
is_traffic_light,
m_edge_based_node_container.GetAnnotation(edge_data1.annotation_data)
.is_left_hand_driving,
// source info
edge_data1.flags.restricted,
m_edge_based_node_container.GetAnnotation(edge_data1.annotation_data).travel_mode,
edge_data1.flags.road_classification.IsMotorwayClass(),
@@ -677,6 +682,7 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
edge_data1.flags.highway_turn_classification,
edge_data1.flags.access_turn_classification,
edge_data1.flags.speed,
// target info
edge_data2.flags.restricted,
m_edge_based_node_container.GetAnnotation(edge_data2.annotation_data).travel_mode,
edge_data2.flags.road_classification.IsMotorwayClass(),
@@ -685,6 +691,7 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
edge_data2.flags.highway_turn_classification,
edge_data2.flags.access_turn_classification,
edge_data2.flags.speed,
// connected roads
road_legs_on_the_right,
road_legs_on_the_left);

2 changes: 0 additions & 2 deletions src/extractor/extractor_callbacks.cpp
Original file line number Diff line number Diff line change
@@ -427,7 +427,6 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
parsed_way.is_startpoint,
parsed_way.forward_restricted,
road_classification,
// @CHAUTODO
parsed_way.highway_turn_classification,
parsed_way.access_turn_classification,
speed}};
@@ -466,7 +465,6 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti
parsed_way.is_startpoint,
parsed_way.backward_restricted,
road_classification,
// @CHAUTODO
parsed_way.highway_turn_classification,
parsed_way.access_turn_classification,
speed}};
6 changes: 4 additions & 2 deletions src/extractor/scripting_environment_lua.cpp
Original file line number Diff line number Diff line change
@@ -348,9 +348,11 @@ void Sol2ScriptingEnvironment::InitContext(LuaScriptingContext &context)
sol::property([](const ExtractionWay &way) { return way.is_left_hand_driving; },
[](ExtractionWay &way, bool flag) { way.is_left_hand_driving = flag; }),
"highway_turn_classification",
&ExtractionWay::highway_turn_classification,
sol::property([](const ExtractionWay &way) { return way.highway_turn_classification; },
[](ExtractionWay &way, int flag) { way.highway_turn_classification = flag; }),
"access_turn_classification",
&ExtractionWay::access_turn_classification);
sol::property([](const ExtractionWay &way) { return way.access_turn_classification; },
[](ExtractionWay &way, int flag) { way.access_turn_classification = flag; }));

auto getTypedRefBySol = [](const sol::object &obj) -> ExtractionRelation::OsmIDTyped {
if (obj.is<osmium::Way>())