Implements Mapping for NodeBasedGraph -> EdgeBasedGraph Translation by daniel-j-h · Pull Request #3645 · Project-OSRM/osrm-backend

daniel-j-h · 2017-02-02T15:59:36Z

Implements a mapping to translate from the node based graph to the edge based graph. This allows us to use our node based graph partition for the edge based graph.

Based on the outline in: #3205 (comment) - @TheMarex care to have a look?

TheMarex · 2017-02-02T19:28:15Z

src/partition/partitioner.cpp

+    }
+
+  private:
+    std::unordered_map<EdgeID, NodeBasedNodes> heads;


The edge_ids are continuous if you merge heads and tails into one array. If the information about head/tail is needed, saving that in a continuous std::vector<bool> should be more efficient.

TheMarex · 2017-02-02T19:29:15Z

src/partition/partitioner.cpp

+            const auto tail = reader.ReadOne<EdgeID>(); // edge based graph backward nodes
+
+            heads.insert({head, {u, v}});
+            tails.insert({tail, {u, v}});


For tail it should be v, u.

TheMarex · 2017-02-02T19:38:25Z

include/extractor/edge_based_graph_factory.hpp

+    struct Mapping
+    {
+        NodeID u, v;
+        EdgeID head, tail;


head and tail can be a little bit confusing here because they typically reference the start/target for an arc (edge based edge). However there is no arc from tail to head and both can be heads and tails for different arcs (there are typically arcs both from and to head and tail).

{forward,backward}_edge_based_node might be more precise but maybe you have another idea for a name.

daniel-j-h · 2017-02-03T15:05:25Z

@TheMarex care to explain what's going on in the contractor in detail. First try is here and is not yet handling all the edge cases the contractor does since I don't understand all of them.

MoKob

I have a few questions, that depending on the answer might be major concerns.

MoKob · 2017-02-14T11:33:27Z

src/extractor/edge_based_graph_factory.cpp

                if (edge_data.edge_id == SPECIAL_NODEID)
                {
-                    InsertEdgeBasedNode(node_v, node_u);
+                    mapping = InsertEdgeBasedNode(node_v, node_u);


hm. This mapping is kind of hanging in the air for me. @daniel-j-h do you see a way we could do this in a less hacky approach? Interleaving it with the EdgeBasedGraph Generation seems a bit messy.

I know and the whole implementation of it just felt wrong as well. I see no other way to hook into the edge based graph factory, though. If you have better ideas, I'm open for changes here.

MoKob · 2017-02-14T11:34:10Z

src/extractor/edge_based_graph_factory.cpp


    BOOST_ASSERT(current_edge_source_coordinate_id == node_v);
+
+    return Mapping{node_u, node_v, forward_data.edge_id, reverse_data.edge_id};


see below. I feel this could become kind of messy. Its a mapping tailored to MLD but integrated into our general data structures.

MoKob · 2017-02-14T11:38:21Z

include/partition/edge_based_graph_reader.hpp

+namespace partition
+{
+
+struct EdgeBasedGraphEdgeData


is this a duplication of the edge based graph edge? Without looking at the rest of the code, I am a bit surprised to see this here. Given the bit restrictions, I strongly would prefer having a single fixed location for this.

Well I started out only exposing some member attributes. Then had to add some for the edge based graph preparation step. Then added the boundary arc flag (which we're currently not using). And finally added all extractor edge members in addition, since we have to serialize out the full edge based graph after modification anyway, right?

…ition

zhudelong · 2021-07-01T03:06:19Z

It seems that the mapping does not consider the new feature, namely multi-via-way restriction. The mapping for duplicated nodes is currently not considered. This leads to a large partition cell and huge memory consumption. This bug should be fixed.

mjjbell · 2021-07-01T08:18:36Z

@zhudelong are you able to provide details of your setup so that I can try and reproduce the issue?

zhudelong · 2021-07-09T07:28:10Z

@mjjbell

osrm-backend/src/partitioner/partitioner.cpp

Line 103 in f7478ba

    
           std::vector<NodeID> edge_based_partition_ids(edge_based_graph.GetNumberOfNodes(),

Can you check the EBG partition result after this mapping process? You can sort the EBG cells according to the cell size, and check whether there exist large cells and whether the largest cell owns a partition id of SPECIAL_NODEID.

If there exists a cell that holds SPECIAL_NODEID, it means some EBG nodes are not mapped during the above process.
According to my test, this issue is caused by the generation of the cnbg_to_ebg file, in which the duplicated nodes are not considered.

mjjbell · 2021-07-09T09:14:19Z

Thanks for the explanation @zhudelong, I see the problem now. I will create a new ticket to track this issue.

daniel-j-h added the Work In Progress label Feb 2, 2017

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch 2 times, most recently from ad9ea6e to fd15824 Compare February 2, 2017 16:55

TheMarex reviewed Feb 2, 2017

View reviewed changes

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch from fd15824 to 75a62c0 Compare February 3, 2017 11:23

MoKob force-pushed the tools/partitionining/graph_view branch 3 times, most recently from bd53c72 to edd9cad Compare February 6, 2017 09:21

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch from 6cee83c to 35c981a Compare February 8, 2017 10:30

MoKob force-pushed the tools/partitionining/graph_view branch from 800f49d to 1c0bc9d Compare February 8, 2017 14:09

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch 2 times, most recently from e76ff22 to d56f201 Compare February 8, 2017 16:30

MoKob force-pushed the tools/partitionining/graph_view branch from 6cee570 to d4b2bff Compare February 13, 2017 10:25

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch from 59e9a04 to a390f68 Compare February 14, 2017 11:10

MoKob force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch from a390f68 to c653fde Compare February 14, 2017 11:20

MoKob suggested changes Feb 14, 2017

View reviewed changes

MoKob force-pushed the tools/partitionining/graph_view branch 2 times, most recently from dfbe7ba to 0b5fbaa Compare February 14, 2017 15:19

daniel-j-h added 7 commits February 14, 2017 18:10

Implements Mapping for NodeBasedGraph -> EdgeBasedgraph Translation

62e7e96

Split Partitioner I/O off into separate headers

387808c

Loads the edge based graph edges and constructs a dynamic graph from it

5024f6c

First try at artificial nodes

fd87572

Store flag for artificial bounary edges and walk border nodes in ebg

4b2ed79

Sigh. Just randomly pick a side for border nodes for now..

cfe6d6b

Re-use the extractor edge based edge for loading the ebg in osrm-part…

b023a76

…ition

daniel-j-h force-pushed the partitioner-for-moritz-nbg-ebg-mapping branch from f35cff6 to b023a76 Compare February 14, 2017 17:14

daniel-j-h merged commit b023a76 into tools/partitionining/graph_view Feb 14, 2017

MoKob deleted the partitioner-for-moritz-nbg-ebg-mapping branch February 15, 2017 09:32

mjjbell mentioned this pull request Jul 17, 2021

Generation of inefficient MLD partitions #6084

Closed


		BOOST_ASSERT(current_edge_source_coordinate_id == node_v);

		return Mapping{node_u, node_v, forward_data.edge_id, reverse_data.edge_id};

Uh oh!

Conversation

daniel-j-h commented Feb 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-j-h commented Feb 3, 2017

Uh oh!

MoKob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhudelong commented Jul 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjjbell commented Jul 1, 2021

Uh oh!

zhudelong commented Jul 9, 2021

Uh oh!

mjjbell commented Jul 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zhudelong commented Jul 1, 2021 •

edited

Loading