fix: transitive closure was not testing correctness

test/Jamfile.v2

@@ -34,8 +34,7 @@ alias graph_test_regular :
     [ run labeled_graph.cpp ]
     [ run finish_edge_bug.cpp ]
 
-    [ run transitive_closure_test.cpp /boost/timer//boost_timer ]
-    [ run transitive_closure_test2.cpp ]
+    [ run transitive_closure_test.cpp ]
     [ compile adj_list_cc.cpp ]
 
     #[ run adj_list_invalidation.cpp ]

test/transitive_closure_test.cpp

@@ -1,178 +1,167 @@
 // Copyright (C) 2001 Vladimir Prus <ghost@cs.msu.su>
 // Copyright (C) 2001 Jeremy Siek <jsiek@cs.indiana.edu>
+// Copyright (c) 2026 Arnaud Becheler
 // 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)
 
 #include <iostream>
-#include <cstdlib>
+#include <random>
+#include <utility>
+#include <vector>
 
+#include <boost/core/lightweight_test.hpp>
 #include <boost/graph/vector_as_graph.hpp>
 #include <boost/graph/adjacency_list.hpp>
-#include <boost/graph/adjacency_list_io.hpp>
 #include <boost/graph/graph_utility.hpp>
 #include <boost/graph/transitive_closure.hpp>
-#include <boost/timer/timer.hpp>
 
-using namespace std;
-using namespace boost;
+template < class Graph >
+using vertex_iter_t = typename boost::graph_traits< Graph >::vertex_iterator;
+
+template < class Graph >
+using out_edge_iter_t = typename boost::graph_traits< Graph >::out_edge_iterator;
+
+template < class Graph >
+using adjacency_iter_t = typename boost::graph_traits< Graph >::adjacency_iterator;
+
+template < class Graph >
+using vertex_desc_t = typename boost::graph_traits< Graph >::vertex_descriptor;
+
+template < class Graph >
+using edge_desc_t = typename boost::graph_traits< Graph >::edge_descriptor;
 
-void generate_graph(int n, double p, vector< vector< int > >& r1)
+template < class Graph >
+using degree_size_t = typename boost::graph_traits< Graph >::degree_size_type;
+
+template < class Generator >
+void generate_graph(int n, double p, std::vector< std::vector< int > >& r1, Generator& uniform01)
 {
-    static class
-    {
-    public:
-        double operator()() { return double(rand()) / RAND_MAX; }
-    } gen;
     r1.clear();
     r1.resize(n);
     for (int i = 0; i < n; ++i)
         for (int j = 0; j < n; ++j)
-            if (gen() < p)
+            if (uniform01() < p)
                 r1[i].push_back(j);
 }
 
 template < class Graph >
-typename graph_traits< Graph >::degree_size_type num_incident(
-    typename graph_traits< Graph >::vertex_descriptor u,
-    typename graph_traits< Graph >::vertex_descriptor v, const Graph& g)
+degree_size_t< Graph > num_incident(vertex_desc_t< Graph > u, vertex_desc_t< Graph > v, const Graph& g)
 {
-    typename graph_traits< Graph >::degree_size_type d = 0;
-    typename graph_traits< Graph >::out_edge_iterator i, i_end;
-    for (boost::tie(i, i_end) = out_edges(u, g); i != i_end; ++i)
+    using boost::target; // ADL also finds std::target for pair edges
+    degree_size_t< Graph > d = 0;
+    out_edge_iter_t< Graph > i, i_end;
+    for (boost::tie(i, i_end) = boost::out_edges(u, g); i != i_end; ++i)
         if (target(*i, g) == v)
             ++d;
     return d;
 }
 
-// (i,j) is in E' iff j is reachable from i
-// Hmm, is_reachable does not detect when there is a non-trivial path
-// from i to i. It always returns true for is_reachable(i,i).
-// This needs to be fixed/worked around.
+// true iff g has a path of at least one edge from u to v. is_reachable treats u
+// as reachable from itself, so a cycle through u is detected via its neighbors.
+template < typename Graph >
+bool has_nontrivial_path(Graph& g, vertex_desc_t< Graph > u, vertex_desc_t< Graph > v)
+{
+    auto reaches = [&](vertex_desc_t< Graph > from, vertex_desc_t< Graph > to) {
+        std::vector< boost::default_color_type > color(boost::num_vertices(g));
+        auto index_map = boost::get(boost::vertex_index, g);
+        auto color_map = boost::make_iterator_property_map(color.begin(), index_map);
+        return boost::is_reachable(from, to, g, color_map);
+    };
+
+    if (u != v)
+        return reaches(u, v);
+
+    adjacency_iter_t< Graph > k, k_end;
+    for (boost::tie(k, k_end) = boost::adjacent_vertices(u, g); k != k_end; ++k)
+        if (reaches(*k, u))
+            return true;
+    return false;
+}
+
+// tc must contain edge (i, j) exactly when g has a nontrivial path from i to j,
+// and never as a duplicate (num_incident, not mere presence)
 template < typename Graph, typename GraphTC >
 bool check_transitive_closure(Graph& g, GraphTC& tc)
 {
-    typename graph_traits< Graph >::vertex_iterator i, i_end;
-    for (boost::tie(i, i_end) = vertices(g); i != i_end; ++i)
+    vertex_iter_t< Graph > i, i_end;
+    for (boost::tie(i, i_end) = boost::vertices(g); i != i_end; ++i)
     {
-        typename graph_traits< Graph >::vertex_iterator j, j_end;
-        for (boost::tie(j, j_end) = vertices(g); j != j_end; ++j)
+        vertex_iter_t< Graph > j, j_end;
+        for (boost::tie(j, j_end) = boost::vertices(g); j != j_end; ++j)
         {
-            bool g_has_edge;
-            typename graph_traits< Graph >::edge_descriptor e_g;
-            typename graph_traits< Graph >::degree_size_type num_tc;
-            boost::tie(e_g, g_has_edge) = edge(*i, *j, g);
-            num_tc = num_incident(*i, *j, tc);
-            if (*i == *j)
-            {
-                if (g_has_edge)
-                {
-                    if (num_tc != 1)
-                        return false;
-                }
-                else
-                {
-                    bool can_reach = false;
-                    typename graph_traits< Graph >::adjacency_iterator k, k_end;
-                    for (boost::tie(k, k_end) = adjacent_vertices(*i, g);
-                         k != k_end; ++k)
-                    {
-                        std::vector< default_color_type > color_map_vec(
-                            num_vertices(g));
-                        if (is_reachable(*k, *i, g,
-                                boost::make_iterator_property_map(
-                                    color_map_vec.begin(),
-                                    get(boost::vertex_index, g))))
-                        {
-                            can_reach = true;
-                            break;
-                        }
-                    }
-                    if (can_reach)
-                    {
-                        if (num_tc != 1)
-                        {
-                            std::cout << "1. " << *i << std::endl;
-                            return false;
-                        }
-                    }
-                    else
-                    {
-                        if (num_tc != 0)
-                        {
-                            std::cout << "2. " << *i << std::endl;
-                            return false;
-                        }
-                    }
-                }
-            }
-            else
-            {
-                std::vector< default_color_type > color_map_vec(
-                    num_vertices(g));
-                if (is_reachable(*i, *j, g,
-                        boost::make_iterator_property_map(color_map_vec.begin(),
-                            get(boost::vertex_index, g))))
-                {
-                    if (num_tc != 1)
-                        return false;
-                }
-                else
-                {
-                    if (num_tc != 0)
-                        return false;
-                }
-            }
+            degree_size_t< Graph > expected = has_nontrivial_path(g, *i, *j) ? 1 : 0;
+            if (num_incident(*i, *j, tc) != expected)
+                return false;
         }
     }
     return true;
 }
 
-bool test(int n, double p)
+// verifies transitive_closure agrees with is_reachable
+// if both are bugged, it would pass
+template < class Generator >
+bool is_random_graph_closure_correct(int n, double p, Generator& uniform01)
 {
-    vector< vector< int > > g1, g1_tc;
-    generate_graph(n, p, g1);
-    cout << "Created graph with " << n << " vertices.\n";
+    std::vector< std::vector< int > > g1, g1_tc;
+    generate_graph(n, p, g1, uniform01);
 
-    vector< vector< int > > g1_c(g1);
+    boost::transitive_closure(g1, g1_tc);
 
-    {
-        boost::timer::auto_cpu_timer t;
-        cout << "transitive_closure" << endl;
-        transitive_closure(
-            g1, g1_tc, vertex_index_map(get(boost::vertex_index, g1)));
-    }
-
-    if (check_transitive_closure(g1, g1_tc))
+    if (check_transitive_closure(g1, g1_tc)){
         return true;
-    else
-    {
-        cout << "Original graph was ";
-        print_graph(g1, get(boost::vertex_index, g1));
-        cout << "Result is ";
-        print_graph(g1_tc, get(boost::vertex_index, g1_tc));
+    } else {
+        std::cout << "failing graph: ";
+        boost::print_graph(g1, boost::get(boost::vertex_index, g1));
+        std::cout << "computed closure: ";
+        boost::print_graph(g1_tc, boost::get(boost::vertex_index, g1_tc));
         return false;
     }
 }
 
+// closure of a fixed graph checked against a hand-computed edge set
+// oracle independent of is_reachable
+bool is_fixed_graph_closure_correct()
+{
+    using graph_t = boost::adjacency_list<>;
+    graph_t g(5), g_tc;
+    boost::add_edge(0, 2, g);
+    boost::add_edge(1, 0, g);
+    boost::add_edge(1, 2, g);
+    boost::add_edge(1, 4, g);
+    boost::add_edge(3, 0, g);
+    boost::add_edge(3, 2, g);
+    boost::add_edge(4, 2, g);
+    boost::add_edge(4, 3, g);
+
+    boost::transitive_closure(g, g_tc);
+
+    constexpr std::pair< int, int > expected[]
+        = { { 0, 2 }, { 1, 0 }, { 1, 2 }, { 1, 3 }, { 1, 4 }, { 3, 0 },
+              { 3, 2 }, { 4, 0 }, { 4, 2 }, { 4, 3 } };
+
+    if (boost::num_edges(g_tc) != sizeof(expected) / sizeof(expected[0]))
+        return false;
+    for (const std::pair< int, int >& e : expected)
+        if (!boost::edge(e.first, e.second, g_tc).second)
+            return false;
+    return true;
+}
+
 int main()
 {
-    srand(42);
-    static class
-    {
-    public:
-        double operator()() { return double(rand()) / RAND_MAX; }
-    } gen;
+    std::mt19937 engine(42);
+    std::uniform_real_distribution< double > dist(0.0, 1.0);
+    auto uniform01 = [&] { return dist(engine); };
 
-    for (size_t i = 0; i < 100; ++i)
+    for (std::size_t i = 0; i < 100; ++i)
     {
-        int n = 0 + int(20 * gen());
-        double p = gen();
-        if (!test(n, p))
-        {
-            cout << "Failed." << endl;
-            return 1;
-        }
+        int n = 0 + int(20 * uniform01());
+        double p = uniform01();
+        BOOST_TEST(is_random_graph_closure_correct(n, p, uniform01));
     }
-    cout << "Passed." << endl;
+
+    BOOST_TEST(is_fixed_graph_closure_correct());
+
+    return boost::report_errors();
 }