Feature : Resource Contrained Shortest Pahts to every vertices from source by SimonPiCarter · Pull Request #239 · boostorg/graph

SimonPiCarter · 2021-01-08T21:56:26Z

Closes #238

If formatting or anything is to be changed let me know.

SimonPiCarter · 2021-01-08T21:58:28Z

+        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)
+                list_vertices.push_back(i);
+        else
+            // else only target
+            list_vertices.push_back(t);


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

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 🙏🏽

SimonPiCarter · 2021-01-08T21:59:43Z

+// Copyright Michael Drexl 2005, 2006.
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://boost.org/LICENSE_1_0.txt)


Copyright should be to be changed as the test is from me but I did not know the exact procedure for this.

Just add your name and date, e.g.

//======================================================================= // Copyright (C) 2026 Simon Carter // // Distributed under the Boost Software License, Version 1.0. (See // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) //=======================================================================

SimonPiCarter · 2021-01-11T20:48:26Z

builds seem to timeout on windows for mscv for unknown reason?

Becheler · 2026-06-04T10:52:49Z

Hi @SimonPiCarter ! I am a bit late to the game but reading through your PR.
Are you still around ?
Thanks again for contributing ❤️

SimonPiCarter · 2026-06-04T11:23:36Z

Hello,
I completely forgot about this PR
I am still around let me know if there is anything I can do.

Becheler · 2026-06-04T12:40:08Z

@SimonPiCarter amazing 🚀
There are conflicts, would you mind rebasing your branch on top of develop to resync the last years changes ? Thank you !

SimonPiCarter · 2026-06-04T12:46:04Z

@Becheler i'll look into it probably tomorow or during the weekend.
Hope there are not too many diffs

Becheler · 2026-06-04T13:01:44Z

Amazing thank you ! 😄
My guess is that there should be little to no changes affecting directly your modifications, but let me know if you have any issue 😉

…vertices

SimonPiCarter · 2026-06-05T08:55:28Z

Alright rebase has been done and ci is under way (looking good)

Becheler

Thanks a lot for this amazing work 😄

Overall this is very good. My only concern is that a deviation in API is hiding behind the new requirements: the output map should be indexed by target vertices to avoid users having to parse the flat vector heavily, and the combination of passing a boolean to switch mode + passing s as both source and target is, I fear, a recipe for future disasters.

I added comments, feel free to discuss them and push back 😄

Becheler · 2026-06-05T09:58:47Z

+    BOOST_TEST(testPathExistence(vectPath_l, manual_path() % "2,0", 0, 0));
+    BOOST_TEST(testPathExistence(vectPath_l, manual_path() % "2,1" % "2,0", 0, 1));
+}
+


I would be more comfortable adding a test case to lock the two versions correctness:

Run r_c_shortest_paths_to_all(g, s, ...) once: get all paths to all vertices

For each vertex v in the graph, run r_c_shortest_paths(g, s, v, ...) individually to get paths to just v

Assert that the paths in the to_all result that end at v match the individual result for v

Does it make sense ?
Also I am slowly working towards a "how to write tests" guide, forgive me for the lack of instructions 🙏🏽

Becheler · 2026-06-05T10:07:40Z

+    // 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,


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 ?

Becheler · 2026-06-05T10:11:14Z

+        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)


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 😄

Becheler · 2026-06-05T10:54:54Z

+// Copyright Michael Drexl 2005, 2006.
+// Distributed under the Boost Software License, Version 1.0.
+// (See accompanying file LICENSE_1_0.txt or copy at
+// http://boost.org/LICENSE_1_0.txt)


Just add your name and date, e.g.

//======================================================================= // Copyright (C) 2026 Simon Carter // // Distributed under the Boost Software License, Version 1.0. (See // accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) //=======================================================================

Becheler · 2026-06-05T11:14:31Z

    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_dispatch(const Graph& g,


Great job trying to stick to the original API and reuse implementation, but i think this is a meaningful deviation that motivates a different API:

passing bools to switch modes is a symptom

passing (s, s) is another symptom

the long error prone parsing of the output for all is another symptom

So I would rather go that way:

namespace detail { // single-target implementation used by all four r_c_shortest_paths overloads void r_c_shortest_paths_impl(g, s, t, solutions, containers, b_all_pareto_optimal_solutions, rc, ref, dominance, la, vis); // all-targets implementation used by r_c_shortest_paths_to_all overloads void r_c_shortest_paths_to_all_impl(g, s, paths_map, /* property map keyed by vertex */ rc, ref, dominance, la, vis); // shared: builds labels for all vertices reachable from s void r_c_shortest_paths_expand_labels(g, s, vec_vertex_labels, /* internal label storage */ rc, ref, dominance, la, vis, early_termination_vertex); // optional<vertex_descriptor> } // detail

That would allow users to simplify usage with something like e.g.:

using PathList = std::vector<std::vector<Edge>>; std::map<Vertex, PathList> storage; auto paths_map = boost::make_assoc_property_map(storage); r_c_shortest_paths_to_all(g, get(vertex_index, g), source, paths_map, Resource{0}, REF{}, Dominance{}); auto& paths_to_v2 = get(paths_map, v2); // all pareto-optimal paths to v2 auto& paths_to_v3 = get(paths_map, v3); // all pareto-optimal paths to v3

Happy to help with further details if you feel this is too much work 😄

Becheler · 2026-06-05T11:16:32Z

+        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)
+                list_vertices.push_back(i);
+        else
+            // else only target
+            list_vertices.push_back(t);


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 🙏🏽

Becheler · 2026-06-05T11:20:32Z

+namespace
+{
+///
+/// @brief Structure representig the vertices, containing the id exposition and name


little typo 😛

Becheler · 2026-06-05T11:21:14Z

+typedef boost::adjacency_list<boost::vecS, boost::vecS, boost::undirectedS, vertex_t, edge_t> graph_t;
+typedef boost::graph_traits<graph_t>::vertex_descriptor vertex_desc;
+typedef boost::graph_traits<graph_t>::edge_descriptor edge_desc;
+typedef std::map<size_t, std::map<size_t, vertex_desc> > vertex_map_t;


prefer the using x = y syntax in C++14 😄

Becheler · 2026-06-05T11:27:27Z

+    return os_p;
+}
+
+/// @brief get all paths from a source to all reachable destination


what happens if the a target is not reachable ? With the associative property map output, the user could easily check:

if (storage.count(v) == 0) // v is unreachable from source else auto& paths = get(paths_map, v); // guaranteed non-empty

That could be cleaner than the current flat vector design where users have to scan all paths and deduce reachability from presence/absence of paths ending at v ?

That would be consistent with existing algorithm returning an empty vector when the target is unreachable.

That would also deserve a test for non-regression 👍🏽

SimonPiCarter commented Jan 8, 2021

View reviewed changes

SimonPierreED added 5 commits June 5, 2026 10:12

r_c_shortest_paths : added possibility to get all solutions from all …

2e2f279

…vertices

r_c_shortest_paths : added interface to get all paths to all vertices

daea3a2

added test on r_c_shortest_paths_to_all

32cd797

added prior to C++11 compatibility

7135fa0

fixed compiler 98 compatibility

0cb8bb8

SimonPiCarter force-pushed the feature/r_c_shortest_paths_to_n branch from 03ea9ba to 0cb8bb8 Compare June 5, 2026 08:17

SimonPiCarter requested review from Becheler and jeremy-murphy as code owners June 5, 2026 08:17

fixed merge

349e95b

Becheler requested changes Jun 5, 2026

View reviewed changes

Conversation

SimonPiCarter commented Jan 8, 2021

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!

SimonPiCarter commented Jan 11, 2021

Uh oh!

Becheler commented Jun 4, 2026

Uh oh!

SimonPiCarter commented Jun 4, 2026

Uh oh!

Becheler commented Jun 4, 2026

Uh oh!

SimonPiCarter commented Jun 4, 2026

Uh oh!

Becheler commented Jun 4, 2026

Uh oh!

SimonPiCarter commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Becheler 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SimonPiCarter commented Jun 5, 2026 •

edited

Loading