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

Refactor update routines #3809

Merged
merged 18 commits into from
Mar 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
- Shared memory notification via conditional variables on Linux or semaphore queue on OS X and Windows with a limit of 128 OSRM Engine instances
- Files
- .osrm.datasource_index file was removed. Data is now part of .osrm.geometries.
- .osrm.edge_lookup was removed. The option `--generate-edge-lookup` does nothing now.
- `osrm-contract` does not depend on the `.osrm.fileIndex` file anymore
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the wiki for these changes


# 5.6.3
- Changes from 5.6.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Feature: osrm-contract command line option: edge-weight-updates-over-factor
Scenario: Logging weight with updates over factor of 2, long segment
When I run "osrm-extract --profile {profile_file} {osm_file} --generate-edge-lookup"
When I run "osrm-contract --edge-weight-updates-over-factor 2 --segment-speed-file {speeds_file} {processed_file}"
And stderr should contain "weight updates"
And stderr should contain "New speed: 100 kph"
And stderr should contain "Segment: 1,2"
And stderr should contain "Segment: 1,3"
And I route I should get
| from | to | route | speed |
| a | b | ab,ab | 100 km/h |
Expand Down
28 changes: 0 additions & 28 deletions features/options/contract/invalid.feature

This file was deleted.

3 changes: 0 additions & 3 deletions features/options/extract/help.feature
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ Feature: osrm-extract command line options: help
And stdout should contain "Configuration:"
And stdout should contain "--profile"
And stdout should contain "--threads"
And stdout should contain "--generate-edge-lookup"
And stdout should contain "--small-component-size"
And it should exit successfully

Expand All @@ -28,7 +27,6 @@ Feature: osrm-extract command line options: help
And stdout should contain "Configuration:"
And stdout should contain "--profile"
And stdout should contain "--threads"
And stdout should contain "--generate-edge-lookup"
And stdout should contain "--small-component-size"
And it should exit successfully

Expand All @@ -42,6 +40,5 @@ Feature: osrm-extract command line options: help
And stdout should contain "Configuration:"
And stdout should contain "--profile"
And stdout should contain "--threads"
And stdout should contain "--generate-edge-lookup"
And stdout should contain "--small-component-size"
And it should exit successfully
4 changes: 2 additions & 2 deletions features/support/shared_steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ module.exports = function () {
let a_type = k.slice(2);
if (whitelist.indexOf(a_type) == -1)
return cb(new Error('Unrecognized annotation field', a_type));
if (!annotation[a_type])
if (annotation && !annotation[a_type])
return cb(new Error('Annotation not found in response', a_type));
got[k] = annotation[a_type];
got[k] = annotation && annotation[a_type] || '';
Copy link
Member

Choose a reason for hiding this comment

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

Hm unrelated changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed this while debugging a failing test. Cucumber crashes instead of just failing the test case.

}
});

Expand Down
3 changes: 3 additions & 0 deletions include/contractor/graph_contractor_adaptors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ std::vector<ContractorEdge> adaptToContractorInput(InputEdgeContainer input_edge

for (const auto &input_edge : input_edge_list)
{
if (input_edge.data.weight == INVALID_EDGE_WEIGHT)
continue;
Copy link
Member

Choose a reason for hiding this comment

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

Wait this is a bug in master? Should this be backported to the 5.6 branch?

Copy link
Member Author

@TheMarex TheMarex Mar 16, 2017

Choose a reason for hiding this comment

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

No previously we filtered these edges out the edge loading routine. We were loading one struct at a time, so we could just skip them.


#ifndef NDEBUG
const unsigned int constexpr DAY_IN_DECI_SECONDS = 24 * 60 * 60 * 10;
if (static_cast<unsigned int>(std::max(input_edge.data.weight, 1)) > DAY_IN_DECI_SECONDS)
Expand Down
19 changes: 5 additions & 14 deletions include/engine/datafacade/contiguous_internalmem_datafacade.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,20 +417,11 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade
m_turn_weight_penalties = util::ShM<TurnPenalty, true>::vector(
turn_weight_penalties_ptr,
data_layout.num_entries[storage::DataLayout::TURN_WEIGHT_PENALTIES]);
if (data_layout.num_entries[storage::DataLayout::TURN_DURATION_PENALTIES] == 0)
{ // Fallback to turn weight penalties that are turn duration penalties in deciseconds
m_turn_duration_penalties = util::ShM<TurnPenalty, true>::vector(
turn_weight_penalties_ptr,
data_layout.num_entries[storage::DataLayout::TURN_WEIGHT_PENALTIES]);
}
else
{
auto turn_duration_penalties_ptr = data_layout.GetBlockPtr<TurnPenalty>(
memory_block, storage::DataLayout::TURN_DURATION_PENALTIES);
m_turn_duration_penalties = util::ShM<TurnPenalty, true>::vector(
turn_duration_penalties_ptr,
data_layout.num_entries[storage::DataLayout::TURN_DURATION_PENALTIES]);
}
auto turn_duration_penalties_ptr = data_layout.GetBlockPtr<TurnPenalty>(
memory_block, storage::DataLayout::TURN_DURATION_PENALTIES);
m_turn_duration_penalties = util::ShM<TurnPenalty, true>::vector(
turn_duration_penalties_ptr,
data_layout.num_entries[storage::DataLayout::TURN_DURATION_PENALTIES]);
}

void InitializeGeometryPointers(storage::DataLayout &data_layout, char *memory_block)
Expand Down
39 changes: 2 additions & 37 deletions include/extractor/edge_based_graph_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,37 +46,6 @@ class ScriptingEnvironment;

namespace lookup
{
// Set to 1 byte alignment
#pragma pack(push, 1)
struct SegmentHeaderBlock
{
std::uint32_t num_osm_nodes;
OSMNodeID previous_osm_node_id;
};
#pragma pack(pop)
static_assert(sizeof(SegmentHeaderBlock) == 12, "SegmentHeaderBlock is not packed correctly");

#pragma pack(push, 1)
struct SegmentBlock
{
OSMNodeID this_osm_node_id;
double segment_length;
EdgeWeight segment_weight;
EdgeWeight segment_duration;
};
#pragma pack(pop)
static_assert(sizeof(SegmentBlock) == 24, "SegmentBlock is not packed correctly");

#pragma pack(push, 1)
struct TurnPenaltiesHeader
{
//! the number of penalties in each block
std::uint64_t number_of_penalties;
};
#pragma pack(pop)
static_assert(std::is_trivial<TurnPenaltiesHeader>::value, "TurnPenaltiesHeader is not trivial");
static_assert(sizeof(TurnPenaltiesHeader) == 8, "TurnPenaltiesHeader is not packed correctly");

#pragma pack(push, 1)
struct TurnIndexBlock
{
Expand Down Expand Up @@ -112,12 +81,10 @@ class EdgeBasedGraphFactory
void Run(ScriptingEnvironment &scripting_environment,
const std::string &original_edge_data_filename,
const std::string &turn_lane_data_filename,
const std::string &edge_segment_lookup_filename,
const std::string &turn_weight_penalties_filename,
const std::string &turn_duration_penalties_filename,
const std::string &turn_penalties_index_filename,
const std::string &cnbg_ebg_mapping_path,
const bool generate_edge_lookup);
const std::string &cnbg_ebg_mapping_path);

// The following get access functions destroy the content in the factory
void GetEdgeBasedEdges(util::DeallocatingVector<EdgeBasedEdge> &edges);
Expand Down Expand Up @@ -182,11 +149,9 @@ class EdgeBasedGraphFactory
void GenerateEdgeExpandedEdges(ScriptingEnvironment &scripting_environment,
const std::string &original_edge_data_filename,
const std::string &turn_lane_data_filename,
const std::string &edge_segment_lookup_filename,
const std::string &turn_weight_penalties_filename,
const std::string &turn_duration_penalties_filename,
const std::string &turn_penalties_index_filename,
const bool generate_edge_lookup);
const std::string &turn_penalties_index_filename);

// Mapping betweenn the node based graph u,v nodes and the edge based graph head,tail edge ids.
// Required in the osrm-partition tool to translate from a nbg partition to a ebg partition.
Expand Down
2 changes: 0 additions & 2 deletions include/extractor/extractor_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ struct ExtractorConfig
edge_graph_output_path = basepath + ".osrm.ebg";
rtree_nodes_output_path = basepath + ".osrm.ramIndex";
rtree_leafs_output_path = basepath + ".osrm.fileIndex";
edge_segment_lookup_path = basepath + ".osrm.edge_segment_lookup";
turn_duration_penalties_path = basepath + ".osrm.turn_duration_penalties";
turn_weight_penalties_path = basepath + ".osrm.turn_weight_penalties";
turn_penalties_index_path = basepath + ".osrm.turn_penalties_index";
Expand Down Expand Up @@ -109,7 +108,6 @@ struct ExtractorConfig

bool generate_edge_lookup;
std::string turn_penalties_index_path;
std::string edge_segment_lookup_path;

bool use_metadata;
};
Expand Down
90 changes: 63 additions & 27 deletions include/extractor/segment_data_container.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,36 +63,71 @@ template <bool UseShareMemory> class SegmentDataContainerImpl
{
}

// TODO these random access functions can be removed after we implemented #3737
auto &ForwardDuration(const DirectionalGeometryID id, const SegmentOffset offset)
auto GetForwardGeometry(const DirectionalGeometryID id)
{
return fwd_durations[index[id] + 1 + offset];
const auto begin = nodes.begin() + index[id];
const auto end = nodes.begin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}
auto &ReverseDuration(const DirectionalGeometryID id, const SegmentOffset offset)

auto GetReverseGeometry(const DirectionalGeometryID id)
{
return boost::adaptors::reverse(GetForwardGeometry(id));
}
Copy link
Member

Choose a reason for hiding this comment

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

Beautiful.


auto GetForwardDurations(const DirectionalGeometryID id)
{
return rev_durations[index[id] + offset];
const auto begin = fwd_durations.begin() + index[id] + 1;
const auto end = fwd_durations.begin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}

auto GetReverseDurations(const DirectionalGeometryID id)
{
const auto begin = rev_durations.begin() + index[id];
const auto end = rev_durations.begin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}
auto &ForwardWeight(const DirectionalGeometryID id, const SegmentOffset offset)

auto GetForwardWeights(const DirectionalGeometryID id)
{
return fwd_weights[index[id] + 1 + offset];
const auto begin = fwd_weights.begin() + index[id] + 1;
const auto end = fwd_weights.begin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}
auto &ReverseWeight(const DirectionalGeometryID id, const SegmentOffset offset)

auto GetReverseWeights(const DirectionalGeometryID id)
{
return rev_weights[index[id] + offset];
const auto begin = rev_weights.begin() + index[id];
const auto end = rev_weights.begin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}
auto &ForwardDatasource(const DirectionalGeometryID id, const SegmentOffset offset)

auto GetForwardDatasources(const DirectionalGeometryID id)
{
return datasources[index[id] + 1 + offset];
const auto begin = datasources.begin() + index[id] + 1;
const auto end = datasources.begin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}
auto &ReverseDatasource(const DirectionalGeometryID id, const SegmentOffset offset)

auto GetReverseDatasources(const DirectionalGeometryID id)
{
return datasources[index[id] + offset];
const auto begin = datasources.begin() + index[id];
const auto end = datasources.begin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}

auto GetForwardGeometry(const DirectionalGeometryID id) const
{
const auto begin = nodes.cbegin() + index.at(id);
const auto end = nodes.cbegin() + index.at(id + 1);
const auto begin = nodes.cbegin() + index[id];
const auto end = nodes.cbegin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}
Expand All @@ -104,52 +139,53 @@ template <bool UseShareMemory> class SegmentDataContainerImpl

auto GetForwardDurations(const DirectionalGeometryID id) const
{
const auto begin = fwd_durations.cbegin() + index.at(id) + 1;
const auto end = fwd_durations.cbegin() + index.at(id + 1);
const auto begin = fwd_durations.cbegin() + index[id] + 1;
const auto end = fwd_durations.cbegin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}

auto GetReverseDurations(const DirectionalGeometryID id) const
{
const auto begin = rev_durations.cbegin() + index.at(id);
const auto end = rev_durations.cbegin() + index.at(id + 1) - 1;
const auto begin = rev_durations.cbegin() + index[id];
const auto end = rev_durations.cbegin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}

auto GetForwardWeights(const DirectionalGeometryID id) const
{
const auto begin = fwd_weights.cbegin() + index.at(id) + 1;
const auto end = fwd_weights.cbegin() + index.at(id + 1);
const auto begin = fwd_weights.cbegin() + index[id] + 1;
const auto end = fwd_weights.cbegin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}

auto GetReverseWeights(const DirectionalGeometryID id) const
{
const auto begin = rev_weights.cbegin() + index.at(id);
const auto end = rev_weights.cbegin() + index.at(id + 1) - 1;
const auto begin = rev_weights.cbegin() + index[id];
const auto end = rev_weights.cbegin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}

auto GetForwardDatasources(const DirectionalGeometryID id) const
{
const auto begin = datasources.cbegin() + index.at(id) + 1;
const auto end = datasources.cbegin() + index.at(id + 1);
const auto begin = datasources.cbegin() + index[id] + 1;
const auto end = datasources.cbegin() + index[id + 1];

return boost::make_iterator_range(begin, end);
}

auto GetReverseDatasources(const DirectionalGeometryID id) const
{
const auto begin = datasources.cbegin() + index.at(id);
const auto end = datasources.cbegin() + index.at(id + 1) - 1;
const auto begin = datasources.cbegin() + index[id];
const auto end = datasources.cbegin() + index[id + 1] - 1;

return boost::adaptors::reverse(boost::make_iterator_range(begin, end));
}

auto GetNumberOfGeometries() const { return index.size() - 1; }
auto GetNumberOfSegments() const { return fwd_weights.size(); }

friend void
Expand Down
3 changes: 3 additions & 0 deletions include/partition/edge_based_graph_reader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ splitBidirectionalEdges(const std::vector<extractor::EdgeBasedEdge> &edges)

for (const auto &edge : edges)
{
if (edge.data.weight == INVALID_EDGE_WEIGHT)
continue;
Copy link
Member

Choose a reason for hiding this comment

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

Urgh same here

Copy link
Member Author

@TheMarex TheMarex Mar 16, 2017

Choose a reason for hiding this comment

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

See above.


directed.emplace_back(edge.source,
edge.target,
edge.data.edge_id,
Expand Down
4 changes: 2 additions & 2 deletions include/updater/updater_config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ struct UpdaterConfig final
void UseDefaultOutputNames()
{
edge_based_graph_path = osrm_input_path.string() + ".ebg";
edge_segment_lookup_path = osrm_input_path.string() + ".edge_segment_lookup";
turn_weight_penalties_path = osrm_input_path.string() + ".turn_weight_penalties";
turn_duration_penalties_path = osrm_input_path.string() + ".turn_duration_penalties";
turn_penalties_index_path = osrm_input_path.string() + ".turn_penalties_index";
node_based_graph_path = osrm_input_path.string() + ".nodes";
edge_data_path = osrm_input_path.string() + ".edges";
geometry_path = osrm_input_path.string() + ".geometry";
rtree_leaf_path = osrm_input_path.string() + ".fileIndex";
datasource_names_path = osrm_input_path.string() + ".datasource_names";
Expand All @@ -58,11 +58,11 @@ struct UpdaterConfig final

std::string edge_based_graph_path;

std::string edge_segment_lookup_path;
std::string turn_weight_penalties_path;
std::string turn_duration_penalties_path;
std::string turn_penalties_index_path;
std::string node_based_graph_path;
std::string edge_data_path;
std::string geometry_path;
std::string rtree_leaf_path;

Expand Down
Loading