Skip to content
Open
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
161 changes: 116 additions & 45 deletions include/boost/graph/r_c_shortest_paths.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include <boost/make_shared.hpp>
#include <boost/enable_shared_from_this.hpp>
#include <boost/foreach.hpp>
#include <boost/graph/graph_traits.hpp>
#include <boost/graph/iteration_macros.hpp>
#include <boost/property_map/property_map.hpp>
Expand Down Expand Up @@ -177,6 +178,7 @@ namespace detail
pareto_optimal_solutions,
std::vector< Resource_Container >& pareto_optimal_resource_containers,
bool b_all_pareto_optimal_solutions,
bool b_all_pareto_optimal_solutions_on_all_vertices,
// to initialize the first label/resource container
// and to carry the type information
const Resource_Container& rc, Resource_Extension_Function& ref,
Expand Down Expand Up @@ -369,6 +371,7 @@ namespace detail
}
}
if (!b_all_pareto_optimal_solutions
&& !b_all_pareto_optimal_solutions_on_all_vertices
&& cur_label->resident_vertex == t)
{
// the devil don't sleep
Expand Down Expand Up @@ -430,51 +433,65 @@ namespace detail
cur_label.reset();
}
}
std::list< Splabel > dsplabels = get(vec_vertex_labels, t);
if(!b_all_pareto_optimal_solutions)
{
dsplabels.sort();
}
typename std::list< Splabel >::const_iterator csi = dsplabels.begin();
typename std::list< Splabel >::const_iterator csi_end = dsplabels.end();
// if d could be reached from o
if (!dsplabels.empty())
std::list<typename graph_traits< Graph >::vertex_descriptor> list_vertices;
// add all vertices if asked for
if(b_all_pareto_optimal_solutions_on_all_vertices)
BGL_FORALL_VERTICES_T(i, g, Graph)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is pre-C++11 support, but BGL is now C++14.
You can instead write:

for (auto v : boost::make_iterator_range(vertices(g)))
    list_vertices.push_back(v);

and remove the boost foreach include 😄

list_vertices.push_back(i);
else
// else only target
list_vertices.push_back(t);
Comment on lines +436 to +443
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not satisfied with those lines but could not find a better way to implement this without impacting performance (very slightly) for the old usage.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree, i think this is a symptom of trying to force the expanded case into the single target implementation 😄 Don't hesitate telling me if you disagree with my understanding 🙏🏽


BOOST_FOREACH(typename graph_traits< Graph >::vertex_descriptor vertex, list_vertices)
{
for (; csi != csi_end; ++csi)
std::list< Splabel > dsplabels = get(vec_vertex_labels, vertex);
if(!b_all_pareto_optimal_solutions
&& !b_all_pareto_optimal_solutions_on_all_vertices)
{
std::vector< typename graph_traits< Graph >::edge_descriptor >
cur_pareto_optimal_path;
boost::shared_ptr<
r_c_shortest_paths_label< Graph, Resource_Container > >
p_cur_label = *csi;
pareto_optimal_resource_containers.push_back(
p_cur_label->cumulated_resource_consumption);
while (p_cur_label->num != 0)
dsplabels.sort();
}
typename std::list< Splabel >::const_iterator csi = dsplabels.begin();
typename std::list< Splabel >::const_iterator csi_end = dsplabels.end();
// if d could be reached from o
if (!dsplabels.empty())
{
for (; csi != csi_end; ++csi)
{
cur_pareto_optimal_path.push_back(p_cur_label->pred_edge);
p_cur_label = p_cur_label->p_pred_label;

// assertion b_is_valid beyond this point is not correct if
// the domination function requires resource levels to be
// strictly greater than existing values
//
// Example
// Customers
// id min_arrival max_departure
// 2 0 974
// 3 0 972
// 4 0 964
// 5 678 801
//
// Path A: 2-3-4-5 (times: 0-16-49-84-678)
// Path B: 3-2-4-5 (times: 0-18-51-62-678)
// The partial path 3-2-4 dominates the other partial path
// 2-3-4, though the path 3-2-4-5 does not strictly dominate
// the path 2-3-4-5
std::vector< typename graph_traits< Graph >::edge_descriptor >
cur_pareto_optimal_path;
boost::shared_ptr<
r_c_shortest_paths_label< Graph, Resource_Container > >
p_cur_label = *csi;
pareto_optimal_resource_containers.push_back(
p_cur_label->cumulated_resource_consumption);
while (p_cur_label->num != 0)
{
cur_pareto_optimal_path.push_back(p_cur_label->pred_edge);
p_cur_label = p_cur_label->p_pred_label;

// assertion b_is_valid beyond this point is not correct if
// the domination function requires resource levels to be
// strictly greater than existing values
//
// Example
// Customers
// id min_arrival max_departure
// 2 0 974
// 3 0 972
// 4 0 964
// 5 678 801
//
// Path A: 2-3-4-5 (times: 0-16-49-84-678)
// Path B: 3-2-4-5 (times: 0-18-51-62-678)
// The partial path 3-2-4 dominates the other partial path
// 2-3-4, though the path 3-2-4-5 does not strictly dominate
// the path 2-3-4-5
}
pareto_optimal_solutions.push_back(cur_pareto_optimal_path);
if (!b_all_pareto_optimal_solutions
&& !b_all_pareto_optimal_solutions_on_all_vertices)
break;
}
pareto_optimal_solutions.push_back(cur_pareto_optimal_path);
if (!b_all_pareto_optimal_solutions)
break;
}
}

Expand Down Expand Up @@ -552,7 +569,7 @@ void r_c_shortest_paths(const Graph& g, const VertexIndexMap& vertex_index_map,
Label_Allocator la, Visitor vis)
{
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, t,
pareto_optimal_solutions, pareto_optimal_resource_containers, true, rc,
pareto_optimal_solutions, pareto_optimal_resource_containers, true, false, rc,
ref, dominance, la, vis);
}

Expand Down Expand Up @@ -582,7 +599,7 @@ void r_c_shortest_paths(const Graph& g, const VertexIndexMap& vertex_index_map,
pareto_optimal_solutions;
std::vector< Resource_Container > pareto_optimal_resource_containers;
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, t,
pareto_optimal_solutions, pareto_optimal_resource_containers, false, rc,
pareto_optimal_solutions, pareto_optimal_resource_containers, false, false, rc,
ref, dominance, la, vis);
if (!pareto_optimal_solutions.empty())
{
Expand Down Expand Up @@ -613,7 +630,7 @@ void r_c_shortest_paths(const Graph& g, const VertexIndexMap& vertex_index_map,
const Dominance_Function& dominance)
{
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, t,
pareto_optimal_solutions, pareto_optimal_resource_containers, true, rc,
pareto_optimal_solutions, pareto_optimal_resource_containers, true, false, rc,
ref, dominance, default_r_c_shortest_paths_allocator(),
default_r_c_shortest_paths_visitor());
}
Expand Down Expand Up @@ -642,7 +659,7 @@ void r_c_shortest_paths(const Graph& g, const VertexIndexMap& vertex_index_map,
pareto_optimal_solutions;
std::vector< Resource_Container > pareto_optimal_resource_containers;
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, t,
pareto_optimal_solutions, pareto_optimal_resource_containers, false, rc,
pareto_optimal_solutions, pareto_optimal_resource_containers, false, false, rc,
ref, dominance, default_r_c_shortest_paths_allocator(),
default_r_c_shortest_paths_visitor());
if (!pareto_optimal_solutions.empty())
Expand All @@ -654,6 +671,60 @@ void r_c_shortest_paths(const Graph& g, const VertexIndexMap& vertex_index_map,
}
// r_c_shortest_paths

// r_c_shortest_paths_to_all functions (handle/interface)
// first overload:
// - return all pareto-optimal solutions
// - specify Label_Allocator and Visitor arguments
template < class Graph, class VertexIndexMap, class EdgeIndexMap,
class Resource_Container, class Resource_Extension_Function,
class Dominance_Function, class Label_Allocator, class Visitor >
void r_c_shortest_paths_to_all(const Graph& g, const VertexIndexMap& vertex_index_map,
const EdgeIndexMap& edge_index_map,
typename graph_traits< Graph >::vertex_descriptor s,
// each inner vector corresponds to a pareto-optimal path
std::vector<
std::vector< typename graph_traits< Graph >::edge_descriptor > >&
pareto_optimal_solutions,
std::vector< Resource_Container >& pareto_optimal_resource_containers,
// to initialize the first label/resource container
// and to carry the type information
const Resource_Container& rc, const Resource_Extension_Function& ref,
const Dominance_Function& dominance,
// to specify the memory management strategy for the labels
Label_Allocator la, Visitor vis)
{
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, s,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My understanding is that you are passing the same vertex as both source and target as a necessity to fit the dispatch API. I would rather make this explicit in the type system:

// signature
r_c_shortest_paths_dispatch(g, ..., s,  boost::optional<vertex_descriptor> t, ...)

// usage
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, boost::make_optional(s), ...);

That would also remove a whole class of future bugs, as it makes the intent explicit (t absent means "all vertices") and a future developer adding a third use of t would naturally write if (t) { do_something(*t) } because the type forces them to handle the null case.

Since the dispatch is not public (lives in detail::), we can safely change its interface: users are not supposed to rely on it.

Does it make sense ?

pareto_optimal_solutions, pareto_optimal_resource_containers, true, true, rc,
ref, dominance, la, vis);
}

// second overload:
// - return all pareto-optimal solutions
// - use default Label_Allocator and Visitor
template < class Graph, class VertexIndexMap, class EdgeIndexMap,
class Resource_Container, class Resource_Extension_Function,
class Dominance_Function >
void r_c_shortest_paths_to_all(const Graph& g, const VertexIndexMap& vertex_index_map,
const EdgeIndexMap& edge_index_map,
typename graph_traits< Graph >::vertex_descriptor s,
// each inner vector corresponds to a pareto-optimal path
std::vector<
std::vector< typename graph_traits< Graph >::edge_descriptor > >&
pareto_optimal_solutions,
std::vector< Resource_Container >& pareto_optimal_resource_containers,
// to initialize the first label/resource container
// and to carry the type information
const Resource_Container& rc, const Resource_Extension_Function& ref,
const Dominance_Function& dominance)
{
r_c_shortest_paths_dispatch(g, vertex_index_map, edge_index_map, s, s,
pareto_optimal_solutions, pareto_optimal_resource_containers, true, true, rc,
ref, dominance, default_r_c_shortest_paths_allocator(),
default_r_c_shortest_paths_visitor());
}

// r_c_shortest_paths_to_all

// check_r_c_path function
template < class Graph, class Resource_Container,
class Resource_Extension_Function >
Expand Down
1 change: 1 addition & 0 deletions test/Jamfile.v2
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ alias graph_test_regular :
[ run make_maximal_planar_test.cpp ]
[ run named_vertices_test.cpp ]
[ run r_c_shortest_paths_test.cpp ]
[ run r_c_shortest_paths_to_all_test.cpp ]
[ run rcsp_custom_vertex_id.cpp ]
[ run rcsp_single_solution.cpp ]
[ run is_straight_line_draw_test.cpp ]
Expand Down
Loading
Loading