Skip to content

Commit

Permalink
remove type from more structs, remove asserts
Browse files Browse the repository at this point in the history
  • Loading branch information
emiltin committed Aug 20, 2014
1 parent c775d08 commit b5674dc
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 17 deletions.
1 change: 0 additions & 1 deletion Contractor/EdgeBasedGraphFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,6 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedNodes()
}

BOOST_ASSERT(u < v);
BOOST_ASSERT(edge_data.type != SHRT_MAX);

// Note: edges that end on barrier nodes or on a turn restriction
// may actually be in two distinct components. We choose the smallest
Expand Down
4 changes: 1 addition & 3 deletions DataStructures/ImportEdge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,15 @@ NodeBasedEdge::NodeBasedEdge(NodeID source,
EdgeWeight weight,
bool forward,
bool backward,
short type,
bool roundabout,
bool in_tiny_cc,
bool access_restricted,
TravelMode travel_mode,
bool is_split)
: source(source), target(target), name_id(name_id), weight(weight), type(type),
: source(source), target(target), name_id(name_id), weight(weight),
forward(forward), backward(backward), roundabout(roundabout), in_tiny_cc(in_tiny_cc),
access_restricted(access_restricted), is_split(is_split), travel_mode(travel_mode)
{
BOOST_ASSERT_MSG(type > 0, "negative edge type");
}

bool EdgeBasedEdge::operator<(const EdgeBasedEdge &other) const
Expand Down
2 changes: 0 additions & 2 deletions DataStructures/ImportEdge.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ struct NodeBasedEdge
EdgeWeight weight,
bool forward,
bool backward,
short type,
bool roundabout,
bool in_tiny_cc,
bool access_restricted,
Expand All @@ -52,7 +51,6 @@ struct NodeBasedEdge
NodeID target;
NodeID name_id;
EdgeWeight weight;
short type;
bool forward : 1;
bool backward : 1;
bool roundabout : 1;
Expand Down
4 changes: 1 addition & 3 deletions DataStructures/NodeBasedGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ struct NodeBasedEdgeData
{
NodeBasedEdgeData()
: distance(INVALID_EDGE_WEIGHT), edgeBasedNodeID(SPECIAL_NODEID),
nameID(std::numeric_limits<unsigned>::max()), type(std::numeric_limits<short>::max()),
nameID(std::numeric_limits<unsigned>::max()),
isAccessRestricted(false), shortcut(false), forward(false), backward(false),
roundabout(false), ignore_in_grid(false), travel_mode(TRAVEL_MODE_INACCESSIBLE)
{
Expand All @@ -22,7 +22,6 @@ struct NodeBasedEdgeData
int distance;
unsigned edgeBasedNodeID;
unsigned nameID;
short type;
bool isAccessRestricted : 1;
bool shortcut : 1;
bool forward : 1;
Expand Down Expand Up @@ -91,7 +90,6 @@ NodeBasedDynamicGraphFromImportEdges(int number_of_nodes, std::vector<ImportEdge
edge.data.roundabout = import_edge.roundabout;
edge.data.ignore_in_grid = import_edge.in_tiny_cc;
edge.data.nameID = import_edge.name_id;
edge.data.type = import_edge.type;
edge.data.isAccessRestricted = import_edge.access_restricted;
edge.data.travel_mode = import_edge.travel_mode;

Expand Down
1 change: 0 additions & 1 deletion Extractor/ExtractionContainers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ void ExtractionContainers::PrepareData(const std::string &output_file_name,
edge_iterator->source_coordinate.lon != std::numeric_limits<int>::min())
{
BOOST_ASSERT(edge_iterator->speed != -1);
BOOST_ASSERT(edge_iterator->type >= 0);
edge_iterator->target_coordinate.lat = node_iterator->lat;
edge_iterator->target_coordinate.lon = node_iterator->lon;

Expand Down
7 changes: 0 additions & 7 deletions Util/GraphLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ NodeID readBinaryOSRMGraphFromStream(std::istream &input_stream,

edge_list.reserve(m);
EdgeWeight weight;
short type;
NodeID nameID;
int length;
bool is_roundabout, ignore_in_grid, is_access_restricted, is_split;
Expand Down Expand Up @@ -160,8 +159,6 @@ NodeID readBinaryOSRMGraphFromStream(std::istream &input_stream,
forward = false;
}

BOOST_ASSERT(type >= 0);

// translate the external NodeIDs to internal IDs
auto internal_id_iter = ext_to_int_id_map.find(source);
if (ext_to_int_id_map.find(source) == ext_to_int_id_map.end())
Expand Down Expand Up @@ -195,7 +192,6 @@ NodeID readBinaryOSRMGraphFromStream(std::istream &input_stream,
weight,
forward,
backward,
type,
is_roundabout,
ignore_in_grid,
is_access_restricted,
Expand Down Expand Up @@ -302,7 +298,6 @@ NodeID readBinaryOSRMGraphFromStream(std::istream &input_stream,

edge_list.reserve(m);
EdgeWeight weight;
short type;
NodeID nameID;
int length;
bool is_roundabout, ignore_in_grid, is_access_restricted, is_split;
Expand All @@ -326,8 +321,6 @@ NodeID readBinaryOSRMGraphFromStream(std::istream &input_stream,
BOOST_ASSERT_MSG(weight > 0, "loaded null weight");
BOOST_ASSERT_MSG(0 <= dir && dir <= 2, "loaded bogus direction");

BOOST_ASSERT(type >= 0);

// translate the external NodeIDs to internal IDs
auto internal_id_iter = ext_to_int_id_map.find(source);
if (ext_to_int_id_map.find(source) == ext_to_int_id_map.end())
Expand Down

1 comment on commit b5674dc

@systemed
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure that removing type is a good idea.

At present OSRM's instructions are a bit arbitrary in what they determine as a 'turn' - see #879, #956, #684, #686, #282. Sometimes turns will not be shown in the instructions at all, or sometimes they'll be shown as "continue" even though they're a modest left turn. Conversely, sometimes a bogus turn will be generated just because the road changes name.

One way to fix this (and the way I'm intending to do it at cycle.travel) is to compare types. So, for example, if the route goes from a motorway to a motorway_link, that is by definition a left/right turn, even though the angle will be small. Or if the road changes name, but the angle is 0° and the road type remains the same, that isn't a turn.

By comparing road types, we should also be able to give a penalty when a minor road crosses a major one (without a crossing), which would be great for bike routing (#592).

@DennisOSRM, any thoughts?

Please sign in to comment.