Try fix ideal refiner

james-d-mitchell · Apr 5, 2024 · a86b116 · a86b116
1 parent 00c40fd
commit a86b116
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 35 deletions.
diff --git a/include/libsemigroups/sims.hpp b/include/libsemigroups/sims.hpp
@@ -2259,45 +2259,38 @@ namespace libsemigroups {
   // resolve the issue but I think in the process of writing it out I have the
   // fix. So Im mostly letting you know why the original idea didn't work in the
   // one sided case.
-  class PrunerIdeal {
+  class SimsRefinerIdeals {
    private:
+    using node_type = uint32_t;
     KnuthBendix _knuth_bendix;
 
    public:
-    PrunerIdeal(Presentation<std::string> const& p)
+    explicit SimsRefinerIdeals(Presentation<std::string> const& p)
         : _knuth_bendix(congruence_kind::right, p) {}
 
     bool operator()(Sims1::word_graph_type const& wg) {
       using sims::right_generating_pairs_no_checks;
-      auto [sink, continue_] = word_graph::unique_sink(
-          wg,
-          wg.cbegin_nodes(),
-          wg.cbegin_nodes() + wg.number_of_active_nodes());
 
-      if (!continue_) {
-        // Complete graph with no sink or word graph with 2-sinks or more
-        return false;
-      } else if (sink == UNDEFINED) {
-        // There's no sink currently so we just carry on
-        return true;
-      }
-      // There's a unique sink
+      node_type sink = UNDEFINED;
 
       for (auto const& p : right_generating_pairs_no_checks(wg)) {
         auto const& u = p.first;
         auto const& v = p.second;
-        if (u.size() < v.size()) {
-          if (word_graph::follow_path_no_checks(wg, 0, u.cbegin(), u.cend())
-              == sink) {
-            continue;
+        if (!_knuth_bendix.contains(u, v)) {
+          auto beta
+              = word_graph::follow_path_no_checks(wg, 0, u.cbegin(), u.cend());
+          if (sink == UNDEFINED) {
+            sink = beta;
+          } else if (sink != beta) {
+            return false;
           }
-        } else if (word_graph::follow_path_no_checks(
-                       wg, 0, v.cbegin(), v.cend())
-                   == sink) {
-          continue;
         }
-        if (!_knuth_bendix.contains(u, v)) {
-          return false;
+      }
+      if (sink != UNDEFINED) {
+        for (auto [a, t] : wg.labels_and_targets_no_checks(sink)) {
+          if (t != UNDEFINED && t != sink) {
+            return false;
+          }
         }
       }
       return true;

diff --git a/include/libsemigroups/word-graph.hpp b/include/libsemigroups/word-graph.hpp
@@ -1479,7 +1479,7 @@ namespace libsemigroups {
     // Returns {Node, bool} where the second value indicates whether or not the
     // DFS should continue, so false if there can be no (or too many) sinks in
     // any completion of "wg" and true otherwise.
-    // TODO to tpp
+    // TODO to tpp, or delete
     template <typename Node, typename Iterator>
     [[nodiscard]] std::pair<Node, bool> unique_sink(WordGraph<Node> const& wg,
                                                     Iterator first,

diff --git a/tests/test-sims.cpp b/tests/test-sims.cpp
@@ -3782,7 +3782,7 @@ namespace libsemigroups {
     p.alphabet("ab");
     p.contains_empty_word(true);
 
-    PrunerIdeal ip(p);
+    SimsRefinerIdeals ip(p);
 
     Sims1 s(p);
     s.add_pruner(ip);
@@ -3822,16 +3822,16 @@ namespace libsemigroups {
     KnuthBendix kb(congruence_kind::twosided, p);
     REQUIRE(kb.number_of_classes() == 15);
 
-    PrunerIdeal ip(to_presentation<std::string>(p));
+    SimsRefinerIdeals ip(to_presentation<std::string>(p));
 
     Sims1 s(p);
-    // s.number_of_threads(1).add_pruner(ip);
-    // REQUIRE(s.number_of_congruences(15) == 15);  // correct value is 18
+    s.number_of_threads(1).add_pruner(ip);
+    REQUIRE(s.number_of_congruences(15) == 15);  // correct value is 15
 
-    p  = partition_monoid(3, author::Machine);
-    ip = PrunerIdeal(to_presentation<std::string>(p));
-    s.init(p).add_pruner(ip);
-    REQUIRE(s.number_of_congruences(203) == 5767);  // checked in GAP
+    // p  = partition_monoid(3, author::Machine);
+    // ip = SimsRefinerIdeals(to_presentation<std::string>(p));
+    // s.init(p).add_pruner(ip);
+    // REQUIRE(s.number_of_congruences(203) == 5767);  // checked in GAP
   }
 
   LIBSEMIGROUPS_TEST_CASE("Sims2",
@@ -3843,11 +3843,84 @@ namespace libsemigroups {
     presentation::add_rule(p, "aaa", "bb");
     presentation::add_rule(p, "aab", "ba");
 
-    PrunerIdeal ip(p);
+    ToddCoxeter tc(congruence_kind::twosided, p);
+    REQUIRE(tc.number_of_classes() == 12);
+
+    SimsRefinerIdeals ip(p);
 
     Sims2 s(p);
     s.add_pruner(ip);
-    REQUIRE(s.number_of_congruences(5) == 2);
+    size_t result = 0;
+    s.for_each(3, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 5);
+    result = 0;
+    s.for_each(4, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 7);
+
+    result = 0;
+    s.for_each(5, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 9);
+
+    result = 0;
+    s.for_each(6, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 11);
+
+    result = 0;
+    s.for_each(7, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 12);
+
+    result = 0;
+    s.for_each(8, [&ip, &result](auto const& wg) {
+      if (ip(wg)) {
+        result++;
+      }
+    });
+    REQUIRE(result == 12);
+
+    REQUIRE(s.number_of_congruences(1) == 1);  // computed using GAP
+    REQUIRE(s.number_of_congruences(2) == 3);  // computed using GAP
+
+    auto it = s.cbegin(3);
+    REQUIRE((*it++) == to_word_graph<uint32_t>(4, {{1, 1}, {1, 1}}));
+    REQUIRE((*it++) == to_word_graph<uint32_t>(4, {{1, 2}, {1, 1}, {1, 1}}));
+    REQUIRE((*it++) == to_word_graph<uint32_t>(4, {{1, 2}, {2, 2}, {2, 2}}));
+    REQUIRE(*it
+            == to_word_graph<uint32_t>(4, {{1, 2}, {2, 3}, {3, 3}, {3, 3}}));
+
+    REQUIRE((sims::right_generating_pairs(*it) | rx::to_vector())
+            == std::vector<std::pair<word_type, word_type>>());
+    REQUIRE((*it++)
+            == to_word_graph<uint32_t>(4, {{1, 2}, {3, 2}, {2, 2}, {2, 2}}));
+    REQUIRE((*it++)
+            == to_word_graph<uint32_t>(4, {{1, 2}, {3, 3}, {3, 3}, {3, 3}}));
+
+    REQUIRE(s.number_of_congruences(3) == 5);  // computed using GAP
+
+    REQUIRE(s.number_of_congruences(4) == 7);   // computed using GAP
+    REQUIRE(s.number_of_congruences(5) == 9);   // computed using GAP
+    REQUIRE(s.number_of_congruences(6) == 11);  // computed using GAP
+    REQUIRE(s.number_of_congruences(7) == 12);  // computed using GAP
+    REQUIRE(s.number_of_congruences(8) == 12);  // computed using GAP
   }
 
   // about 2 seconds