perf: improve perf of is_acyclic

Author: james-d-mitchell

include/libsemigroups/detail/path-iterators.hpp

@@ -230,6 +230,8 @@ namespace libsemigroups {
       using iterator_category = std::forward_iterator_tag;
 
      private:
+      // TODO(1) maybe try replacing _can_reach_target with std::unordered_set,
+      // then we can use the output of ancestors_of directly
       std::vector<bool>      _can_reach_target;
       value_type             _edges;
       WordGraph<Node> const* _word_graph;

include/libsemigroups/detail/path-iterators.tpp

@@ -301,37 +301,12 @@ namespace libsemigroups {
     template <typename Node>
     void const_pstilo_iterator<Node>::init_can_reach_target() {
       if (_can_reach_target.empty()) {
-        std::vector<std::vector<node_type>> in_neighbours(
-            _word_graph->number_of_nodes(), std::vector<node_type>({}));
-        for (auto n = _word_graph->cbegin_nodes();
-             n != _word_graph->cend_nodes();
-             ++n) {
-          for (auto e = _word_graph->cbegin_targets(*n);
-               e != _word_graph->cend_targets(*n);
-               ++e) {
-            if (*e != UNDEFINED) {
-              in_neighbours[*e].push_back(*n);
-            }
-          }
-        }
-
         _can_reach_target.resize(_word_graph->number_of_nodes(), false);
-        _can_reach_target[_target]   = true;
-        std::vector<node_type>& todo = in_neighbours[_target];
-        std::vector<node_type>  next;
-
-        while (!todo.empty()) {
-          for (auto& m : todo) {
-            if (_can_reach_target[m] == 0) {
-              _can_reach_target[m] = true;
-              next.insert(next.end(),
-                          in_neighbours[m].cbegin(),
-                          in_neighbours[m].cend());
-            }
-          }
-          std::swap(next, todo);
-          next.clear();
-        }
+        auto ancestors
+            = word_graph::ancestors_of_no_checks(*_word_graph, _target);
+        std::for_each(ancestors.begin(), ancestors.end(), [this](auto n) {
+          _can_reach_target[n] = true;
+        });
       }
     }
 

include/libsemigroups/word-graph.hpp

@@ -2200,6 +2200,10 @@ namespace libsemigroups {
     [[nodiscard]] std::unordered_set<Node1>
     nodes_reachable_from(WordGraph<Node1> const& wg, Node2 source);
 
+    template <typename Node1, typename Node2>
+    [[nodiscard]] std::unordered_set<Node1>
+    ancestors_of(WordGraph<Node1> const& wg, Node2 source);
+
     //! \brief Returns the std::unordered_set of nodes reachable from a given
     //! node in a word graph.
     //!
@@ -2225,6 +2229,10 @@ namespace libsemigroups {
     [[nodiscard]] std::unordered_set<Node1>
     nodes_reachable_from_no_checks(WordGraph<Node1> const& wg, Node2 source);
 
+    template <typename Node1, typename Node2>
+    [[nodiscard]] std::unordered_set<Node1>
+    ancestors_of_no_checks(WordGraph<Node1> const& wg, Node2 source);
+
     //! \brief Returns the number of nodes reachable from a given node in a
     //! word graph.
     //!
@@ -2824,7 +2832,7 @@ namespace libsemigroups {
         // always have an odd number of arguments, so we check that it's even
         // here (the argument x and an odd number of further arguments).
         WordGraph<Node> xy;
-                        operator()(xy, x, std::forward<Args>(args)...);
+        operator()(xy, x, std::forward<Args>(args)...);
         return xy;
       }
 
@@ -2859,7 +2867,7 @@ namespace libsemigroups {
         return is_subrelation(x, static_cast<Node>(0), y, static_cast<Node>(0));
       }
     };  // JoinerMeeterCommon
-  }     // namespace detail
+  }  // namespace detail
 
   //! \ingroup word_graph_group
   //! \brief Class for taking joins of word graphs.

include/libsemigroups/word-graph.tpp

@@ -774,9 +774,10 @@ namespace libsemigroups {
       Node1              next_preorder_num = 0;
       std::vector<Node1> postorder(N, N);
       Node1              next_postorder_num = 0;
-      // TODO(1) there should be a better way of doing this
+
+      auto ancestors = ancestors_of_no_checks(wg, target);
       for (auto n : wg.nodes()) {
-        if (!is_reachable(wg, n, static_cast<Node1>(target))) {
+        if (ancestors.count(n) == 0) {
           preorder[n] = N + 1;
         }
       }
@@ -875,6 +876,55 @@ namespace libsemigroups {
       return nodes_reachable_from_no_checks(wg, source);
     }
 
+    template <typename Node1, typename Node2>
+    std::unordered_set<Node1> ancestors_of_no_checks(WordGraph<Node1> const& wg,
+                                                     Node2 target) {
+      static_assert(sizeof(Node2) <= sizeof(Node1));
+      using label_type = typename WordGraph<Node1>::label_type;
+
+      size_t const N = wg.number_of_nodes();
+      size_t const M = wg.out_degree();
+
+      // Reverse the WordGraph and then just find the nodes reachable from
+      // target in the reversed graph. Since the reverse of a WordGraph is no
+      // longer a WordGraph we use a vector of vectors here. Alternatively, we
+      // could use the technique used in WordGraphWithSources (the sources are
+      // essentially the reversed graph) to create the reversed graph (or just
+      // use it if we know it already, like in ToddCoxeter).
+      std::vector<std::vector<Node1>> in_neighbours(N, std::vector<Node1>({}));
+      for (Node1 s = 0; s < N; ++s) {
+        for (label_type a = 0; a < M; ++a) {
+          auto t = wg.target_no_checks(s, a);
+          if (t != UNDEFINED) {
+            in_neighbours[t].push_back(s);
+          }
+        }
+      }
+
+      std::unordered_set<Node1> seen;
+      std::stack<Node1>         stack;
+      stack.push(target);
+
+      while (!stack.empty()) {
+        Node1 s = stack.top();
+        stack.pop();
+        if (seen.insert(s).second) {
+          for (auto t : in_neighbours[s]) {
+            stack.push(t);
+          }
+        }
+      }
+      return seen;
+    }
+
+    template <typename Node1, typename Node2>
+    std::unordered_set<Node1> ancestors_of(WordGraph<Node1> const& wg,
+                                           Node2                   target) {
+      static_assert(sizeof(Node2) <= sizeof(Node1));
+      throw_if_node_out_of_bounds(wg, static_cast<Node1>(target));
+      return ancestors_of_no_checks(wg, target);
+    }
+
     template <typename Node1, typename Node2, typename Iterator>
     Node1 follow_path(WordGraph<Node1> const& wg,
                       Node2                   from,
@@ -1552,8 +1602,8 @@ namespace libsemigroups {
     _uf.init(xnum_nodes_reachable_from_root + ynum_nodes_reachable_from_root);
     _uf.unite(xroot, yroot + xnum_nodes_reachable_from_root);
 
-    // The stack can't be empty if this function runs to the end so no need to
-    // do anything.
+    // The stack can't be empty if this function runs to the end so no need
+    // to do anything.
     LIBSEMIGROUPS_ASSERT(_stck.empty());
     // 0 .. x.number_of_nodes() - 1, x.number_of_nodes()  ..
     //   x.number_of_nodes() + y.number_of_nodes() -1
@@ -1599,9 +1649,9 @@ namespace libsemigroups {
         ynum_nodes_reachable_from_root,
         yroot);
     _uf.normalize();
-    // It can be that _uf is equivalent to [0, 0, 2] at this point (and there's
-    // no way for it to not be like this, because 2 doesn't belong to the class
-    // of 0), and so we require the following lookup.
+    // It can be that _uf is equivalent to [0, 0, 2] at this point (and
+    // there's no way for it to not be like this, because 2 doesn't belong
+    // to the class of 0), and so we require the following lookup.
     _lookup.resize(xnum_nodes_reachable_from_root);
     LIBSEMIGROUPS_ASSERT(_lookup.size() == xnum_nodes_reachable_from_root);
     std::fill(_lookup.begin(), _lookup.end(), static_cast<Node>(UNDEFINED));

tests/test-todd-coxeter.cpp

@@ -22,6 +22,7 @@
 
 #include "Catch2-3.8.0/catch_amalgamated.hpp"  // for TEST_CASE
 #include "libsemigroups/constants.hpp"
+#include "libsemigroups/word-graph.hpp"
 #include "test-main.hpp"  // for LIBSEMIGROUPS_TEST_CASE
 
 #include "libsemigroups/bmat8.hpp"
@@ -928,6 +929,16 @@ namespace libsemigroups {
 
     REQUIRE(tc.number_of_classes() == 6);
     REQUIRE(index_of(tc, {1}) == index_of(tc, {2}));
+    REQUIRE(tc.word_graph().number_of_nodes() == 7);
+    REQUIRE(tc.word_graph().target(0, 0) == 1);
+    auto        pred = word_graph::ancestors_of_no_checks(tc.word_graph(), 1);
+    std::vector result(pred.begin(), pred.end());
+    std::sort(result.begin(), result.end());
+    REQUIRE(result == std::vector<uint32_t>({0, 1, 2, 3, 4, 5, 6}));
+    auto desc = word_graph::nodes_reachable_from(tc.word_graph(), 1);
+    result.assign(desc.begin(), desc.end());
+    std::sort(result.begin(), result.end());
+    REQUIRE(result == std::vector<uint32_t>({1, 2, 3, 4, 5, 6}));
   }
 
   LIBSEMIGROUPS_TEST_CASE("ToddCoxeter",
@@ -3191,6 +3202,11 @@ namespace libsemigroups {
 
     REQUIRE(H.number_of_classes() == 16'384);
 
+    REQUIRE(word_graph::is_reachable(H.word_graph(), 0, 0));
+    REQUIRE(word_graph::ancestors_of_no_checks(H.word_graph(), 0).size()
+            == 16'384);
+    REQUIRE(!word_graph::is_acyclic(H.word_graph(), 0, 0));
+
     // The following no longer works
     // REQUIRE(class_of(H, "").size_hint() == POSITIVE_INFINITY);
     REQUIRE((class_of(H, "") | rx::take(50) | rx::to_vector())