From 8a608eb930a0153df14909ca7dd90eb65dddd23f Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Wed, 8 Apr 2015 18:53:10 +0200 Subject: [PATCH 01/10] Add some doc to the extractor --- extractor/extraction_way.hpp | 6 ++++++ extractor/extractor_callbacks.cpp | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/extractor/extraction_way.hpp b/extractor/extraction_way.hpp index d47de20b086..d344e366529 100644 --- a/extractor/extraction_way.hpp +++ b/extractor/extraction_way.hpp @@ -34,6 +34,12 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +/** + * This struct is the direct result of the call to ```way_function``` + * in the lua based profile. + * + * It is split into multiple edge segments in the ExtractorCallback. + */ struct ExtractionWay { ExtractionWay() { clear(); } diff --git a/extractor/extractor_callbacks.cpp b/extractor/extractor_callbacks.cpp index 224468b060e..fba9206af75 100644 --- a/extractor/extractor_callbacks.cpp +++ b/extractor/extractor_callbacks.cpp @@ -72,7 +72,15 @@ void ExtractorCallbacks::ProcessRestriction( // "y" : "n"); } } -/** warning: caller needs to take care of synchronization! */ +/** + * This function takes the geometry contained in the ```input_way``` and the tags + * computed by the lua profile inside ```parsed_way``` and computes all edge segments. + * + * Depending on the forward/backwards weights the edges are split into forward and backward + * edges. + * + * warning: caller needs to take care of synchronization! + */ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const ExtractionWay &parsed_way) { if (((0 >= parsed_way.forward_speed) || From 30352192125ad9828b120215ce93e8c0a69060d9 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Thu, 9 Apr 2015 12:26:46 +0200 Subject: [PATCH 02/10] Add further documentation --- extractor/extraction_containers.cpp | 3 +++ extractor/extractor.cpp | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/extractor/extraction_containers.cpp b/extractor/extraction_containers.cpp index 8c484eb8146..8cf1a3b6c40 100644 --- a/extractor/extraction_containers.cpp +++ b/extractor/extraction_containers.cpp @@ -301,6 +301,7 @@ void ExtractionContainers::PrepareData(const std::string &output_file_name, node_iterator = all_nodes_list.begin(); edge_iterator = all_edges_list.begin(); + // Also serializes the edges while (edge_iterator != all_edges_list.end() && node_iterator != all_nodes_list.end()) { if (edge_iterator->target < node_iterator->node_id) @@ -329,6 +330,8 @@ void ExtractionContainers::PrepareData(const std::string &output_file_name, int integer_weight = std::max( 1, (int)std::floor( (edge_iterator->is_duration_set ? edge_iterator->speed : weight) + .5)); + // FIXME: This means we have a _minimum_ edge length of 1m + // maybe use dm as base unit? const int integer_distance = std::max(1, (int)distance); const short zero = 0; const short one = 1; diff --git a/extractor/extractor.cpp b/extractor/extractor.cpp index 0581e4e5cd1..528fe33a6de 100644 --- a/extractor/extractor.cpp +++ b/extractor/extractor.cpp @@ -64,6 +64,24 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include +/** + * TODO: Refactor this function into smaller functions for better readability. + * + * This function is the entry point for the whole extraction process. The goal of the extraction + * step is to filter and convert the OSM geometry to something more fitting for routing. + * That includes: + * - extracting turn restrictions + * - splitting ways into (directional!) edge segments + * - checking if nodes are barriers or traffic signal + * - discarding all tag information: All relevant type information for nodes/ways + * is extracted at this point. + * + * The result of this process are the following files: + * .names : Names of all streets, stored as long consecutive string with prefix sum based index + * .osrm : Nodes and edges in a intermediate format that easy to digest for osrm-prepare + * .restrictions : Turn restrictions that are used my osrm-prepare to construct the edge-expanded graph + * + */ int extractor::run(const ExtractorConfig &extractor_config) { try @@ -83,6 +101,7 @@ int extractor::run(const ExtractorConfig &extractor_config) // setup scripting environment ScriptingEnvironment scripting_environment(extractor_config.profile_path.string().c_str()); + // used to deduplicate street names: actually maps to name ids std::unordered_map string_map; string_map[""] = 0; From 34031aab1b7f4e20c22a7e37b4e266e420811634 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 12:35:24 +0200 Subject: [PATCH 03/10] Add further documentation to ExtractorCallbacks --- extractor/extractor_callbacks.cpp | 15 ++++++++++----- extractor/extractor_callbacks.hpp | 7 +++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/extractor/extractor_callbacks.cpp b/extractor/extractor_callbacks.cpp index fba9206af75..6ae9ff437c6 100644 --- a/extractor/extractor_callbacks.cpp +++ b/extractor/extractor_callbacks.cpp @@ -47,7 +47,12 @@ ExtractorCallbacks::ExtractorCallbacks(ExtractionContainers &extraction_containe { } -/** warning: caller needs to take care of synchronization! */ +/** + * Takes the node position from osmium and the filtered properties from the lua + * profile and saves them to external memory. + * + * warning: caller needs to take care of synchronization! + */ void ExtractorCallbacks::ProcessNode(const osmium::Node &input_node, const ExtractionNode &result_node) { @@ -73,11 +78,11 @@ void ExtractorCallbacks::ProcessRestriction( } } /** - * This function takes the geometry contained in the ```input_way``` and the tags - * computed by the lua profile inside ```parsed_way``` and computes all edge segments. + * Takes the geometry contained in the ```input_way``` and the tags computed + * by the lua profile inside ```parsed_way``` and computes all edge segments. * - * Depending on the forward/backwards weights the edges are split into forward and backward - * edges. + * Depending on the forward/backwards weights the edges are split into forward + * and backward edges. * * warning: caller needs to take care of synchronization! */ diff --git a/extractor/extractor_callbacks.hpp b/extractor/extractor_callbacks.hpp index 8eab0182b1a..79225f4977c 100644 --- a/extractor/extractor_callbacks.hpp +++ b/extractor/extractor_callbacks.hpp @@ -43,6 +43,13 @@ class ExtractionContainers; struct InputRestrictionContainer; struct ExtractionNode; +/** + * This class is uses by the extractor with the results of the + * osmium based parsing and the customization through the lua profile. + * + * It mediates between the multi-threaded extraction process and the external memory containers. + * Thus the synchronization is handled inside of the extractor. + */ class ExtractorCallbacks { private: From 5ff95dc32d8e53ce7ae36ce33951f11761310530 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 12:40:30 +0200 Subject: [PATCH 04/10] Move string_map inside external_callbacks It is not referenced outside this calls, thus the lifetime can be safely handled by it. --- extractor/extractor.cpp | 7 +------ extractor/extractor_callbacks.cpp | 6 +++--- extractor/extractor_callbacks.hpp | 6 +++--- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/extractor/extractor.cpp b/extractor/extractor.cpp index 528fe33a6de..fce0f889844 100644 --- a/extractor/extractor.cpp +++ b/extractor/extractor.cpp @@ -101,13 +101,8 @@ int extractor::run(const ExtractorConfig &extractor_config) // setup scripting environment ScriptingEnvironment scripting_environment(extractor_config.profile_path.string().c_str()); - // used to deduplicate street names: actually maps to name ids - std::unordered_map string_map; - string_map[""] = 0; - ExtractionContainers extraction_containers; - auto extractor_callbacks = - osrm::make_unique(extraction_containers, string_map); + auto extractor_callbacks = osrm::make_unique(extraction_containers); const osmium::io::File input_file(extractor_config.input_path.string()); osmium::io::Reader reader(input_file); diff --git a/extractor/extractor_callbacks.cpp b/extractor/extractor_callbacks.cpp index 6ae9ff437c6..deef831c3e9 100644 --- a/extractor/extractor_callbacks.cpp +++ b/extractor/extractor_callbacks.cpp @@ -41,10 +41,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include -ExtractorCallbacks::ExtractorCallbacks(ExtractionContainers &extraction_containers, - std::unordered_map &string_map) - : string_map(string_map), external_memory(extraction_containers) +ExtractorCallbacks::ExtractorCallbacks(ExtractionContainers &extraction_containers) + : external_memory(extraction_containers) { + string_map[""] = 0; } /** diff --git a/extractor/extractor_callbacks.hpp b/extractor/extractor_callbacks.hpp index 79225f4977c..25bbbaba14f 100644 --- a/extractor/extractor_callbacks.hpp +++ b/extractor/extractor_callbacks.hpp @@ -53,14 +53,14 @@ struct ExtractionNode; class ExtractorCallbacks { private: - std::unordered_map &string_map; + // used to deduplicate street names: actually maps to name ids + std::unordered_map string_map; ExtractionContainers &external_memory; public: ExtractorCallbacks() = delete; ExtractorCallbacks(const ExtractorCallbacks &) = delete; - explicit ExtractorCallbacks(ExtractionContainers &extraction_containers, - std::unordered_map &string_map); + explicit ExtractorCallbacks(ExtractionContainers &extraction_containers); // warning: caller needs to take care of synchronization! void ProcessNode(const osmium::Node ¤t_node, const ExtractionNode &result_node); From c25d14e454ff1291980f9d334efe8fa9d43f44fd Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 12:57:30 +0200 Subject: [PATCH 05/10] Remove unnecessary header include --- extractor/restriction_parser.cpp | 3 +-- extractor/restriction_parser.hpp | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/extractor/restriction_parser.cpp b/extractor/restriction_parser.cpp index ea9cad27a23..3a8c0ed07f0 100644 --- a/extractor/restriction_parser.cpp +++ b/extractor/restriction_parser.cpp @@ -27,7 +27,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "restriction_parser.hpp" #include "extraction_way.hpp" -#include "scripting_environment.hpp" #include "../data_structures/external_memory_node.hpp" #include "../util/lua_util.hpp" @@ -53,7 +52,7 @@ int lua_error_callback(lua_State *lua_state) } RestrictionParser::RestrictionParser(lua_State *lua_state) - : /*lua_state(scripting_environment.getLuaState()),*/ use_turn_restrictions(true) + : use_turn_restrictions(true) { ReadUseRestrictionsSetting(lua_state); diff --git a/extractor/restriction_parser.hpp b/extractor/restriction_parser.hpp index 0a632d83ec6..d19b5c55ab0 100644 --- a/extractor/restriction_parser.hpp +++ b/extractor/restriction_parser.hpp @@ -44,7 +44,6 @@ class ScriptingEnvironment; class RestrictionParser { public: - // RestrictionParser(ScriptingEnvironment &scripting_environment); RestrictionParser(lua_State *lua_state); mapbox::util::optional TryParse(const osmium::Relation &relation) const; From 006bcc0fc896074d2077dcd5d78390eea7d9b1db Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 15:33:41 +0200 Subject: [PATCH 06/10] Add some documentation to the restriction parser --- data_structures/restriction.hpp | 3 --- extractor/restriction_parser.cpp | 7 +++++++ extractor/restriction_parser.hpp | 19 ++++++++++++++++++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/data_structures/restriction.hpp b/data_structures/restriction.hpp index b808d3070b0..0676cecc374 100644 --- a/data_structures/restriction.hpp +++ b/data_structures/restriction.hpp @@ -79,9 +79,6 @@ struct TurnRestriction struct InputRestrictionContainer { - // EdgeID fromWay; - // EdgeID toWay; - // NodeID via_node; TurnRestriction restriction; InputRestrictionContainer(EdgeID fromWay, EdgeID toWay, EdgeID vw) diff --git a/extractor/restriction_parser.cpp b/extractor/restriction_parser.cpp index 3a8c0ed07f0..77dae9ce3a9 100644 --- a/extractor/restriction_parser.cpp +++ b/extractor/restriction_parser.cpp @@ -102,6 +102,13 @@ void RestrictionParser::ReadRestrictionExceptions(lua_State *lua_state) } } +/** + * Tries to parse an relation as turn restriction. This can fail for a number of + * reasons, this the return type is a mapbox::util::optional<>. + * + * Some restrictions can also be ignored: See the ```get_exceptions``` function + * in the corresponding profile. + */ mapbox::util::optional RestrictionParser::TryParse(const osmium::Relation &relation) const { diff --git a/extractor/restriction_parser.hpp b/extractor/restriction_parser.hpp index d19b5c55ab0..23b111d2745 100644 --- a/extractor/restriction_parser.hpp +++ b/extractor/restriction_parser.hpp @@ -41,6 +41,24 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct lua_State; class ScriptingEnvironment; +/** + * Parses the relations that represents turn restrictions. + * + * Currently only restrictions where the via objects is a node are supported. + * from via to + * ------->(x)--------> + * + * While this class does not directly invoke any lua code _per relation_ it does + * load configuration values from the profile, that are saved in variables. + * Namely ```use_turn_restrictions``` and ```get_exceptions```. + * + * The restriction is represented by the osm id of the from way, the osm id of the + * to way and the osm id of the via node. This representation must be post-processed + * in the extractor to work with the edge-based data-model of OSRM: + * Since the from and to way share the via-way as node a turn will have the following form: + * ...----(a)-----(via)------(b)----... + * So it can be represented by the tripe (a, via, b). + */ class RestrictionParser { public: @@ -53,7 +71,6 @@ class RestrictionParser void ReadRestrictionExceptions(lua_State *lua_state); bool ShouldIgnoreRestriction(const std::string &except_tag_string) const; - // lua_State *lua_state; std::vector restriction_exceptions; bool use_turn_restrictions; }; From 345dd2481b6aa8d3504e78cef851213e752693bd Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 15:40:49 +0200 Subject: [PATCH 07/10] Add documentation to InputRestrictionContainer --- data_structures/restriction.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/data_structures/restriction.hpp b/data_structures/restriction.hpp index 0676cecc374..a59fb1e26aa 100644 --- a/data_structures/restriction.hpp +++ b/data_structures/restriction.hpp @@ -77,6 +77,13 @@ struct TurnRestriction } }; +/** + * This is just a wrapper around TurnRestriction used in the extractor. + * + * Could be merged with TurnRestriction. For now the type-destiction makes sense + * as the format in which the restriction is presented in the extractor and in the + * preprocessing is different. (see restriction_parser.cpp) + */ struct InputRestrictionContainer { TurnRestriction restriction; From 3b435d8956a94929d2e9b1972085f875e395268f Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 15:46:49 +0200 Subject: [PATCH 08/10] Add documentation to ScriptingEnvironment --- extractor/scripting_environment.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extractor/scripting_environment.hpp b/extractor/scripting_environment.hpp index be05103c8ca..8722aee8fa5 100644 --- a/extractor/scripting_environment.hpp +++ b/extractor/scripting_environment.hpp @@ -35,6 +35,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct lua_State; +/** + * Creates a lua context and binds osmium way, node and relation objects and + * ExtractionWay and ExtractionNode to lua objects. + * + * Each thread has its own lua state which is implemented with thread specific + * storage from TBB. + */ class ScriptingEnvironment { public: From d96e90c6f4b4065baec14e3a3429998ec460f0b1 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 10 Apr 2015 16:10:00 +0200 Subject: [PATCH 09/10] Add documentation to ExtractionContainer --- extractor/extraction_containers.cpp | 11 +++++++++++ extractor/extraction_containers.hpp | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/extractor/extraction_containers.cpp b/extractor/extraction_containers.cpp index 8cf1a3b6c40..7c912f33f3e 100644 --- a/extractor/extraction_containers.cpp +++ b/extractor/extraction_containers.cpp @@ -62,6 +62,17 @@ ExtractionContainers::~ExtractionContainers() way_start_end_id_list.clear(); } +/** + * Processes the collected data and serializes it. + * At this point nodes are still referenced by their OSM id. + * + * - map start-end nodes of ways to ways used int restrictions to compute compressed + * trippe representation + * - filter nodes list to nodes that are referenced by ways + * - merge edges with nodes to include location of start/end points and serialize + * + * FIXME: Each of this step should be an own function for readability. + */ void ExtractionContainers::PrepareData(const std::string &output_file_name, const std::string &restrictions_file_name) { diff --git a/extractor/extraction_containers.hpp b/extractor/extraction_containers.hpp index 12d88a26c89..d04f595496e 100644 --- a/extractor/extraction_containers.hpp +++ b/extractor/extraction_containers.hpp @@ -36,6 +36,10 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include +/** + * Uses external memory containers from stxxl to store all the data that + * is collected by the extractor callbacks. + */ class ExtractionContainers { #ifndef _MSC_VER From fbb4e9078a51086b4062fc624c46f2a0f675494b Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Mon, 20 Apr 2015 10:30:21 +0200 Subject: [PATCH 10/10] Updated restriction parser doc --- extractor/restriction_parser.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extractor/restriction_parser.hpp b/extractor/restriction_parser.hpp index 23b111d2745..f99335d2f48 100644 --- a/extractor/restriction_parser.hpp +++ b/extractor/restriction_parser.hpp @@ -55,7 +55,7 @@ class ScriptingEnvironment; * The restriction is represented by the osm id of the from way, the osm id of the * to way and the osm id of the via node. This representation must be post-processed * in the extractor to work with the edge-based data-model of OSRM: - * Since the from and to way share the via-way as node a turn will have the following form: + * Since the from and to way share the via-node a turn will have the following form: * ...----(a)-----(via)------(b)----... * So it can be represented by the tripe (a, via, b). */