Fix pruner evaluation order bug

reiniscirpons · Apr 9, 2024 · 20001f6 · 20001f6
1 parent 2620f9a
commit 20001f6
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 75 deletions.
diff --git a/src/sims.cpp b/src/sims.cpp
@@ -596,17 +596,18 @@ namespace libsemigroups {
         }
       }
 
-      for (auto const& pruner : _sims1or2->pruners()) {
-        if (!pruner(_felsch_graph)) {
-          return false;
-        }
-      }
       return true;
     }
 
     template <typename Sims1or2>
     bool SimsBase<Sims1or2>::IteratorBase::install_descendents(
         PendingDef const& current) {
+      for (auto const& pruner : _sims1or2->pruners()) {
+        if (!pruner(_felsch_graph)) {
+          return false;
+        }
+      }
+
       letter_type     a        = current.generator + 1;
       size_type const M        = _felsch_graph.number_of_active_nodes();
       size_type const N        = _felsch_graph.number_of_edges();

diff --git a/tests/test-sims.cpp b/tests/test-sims.cpp
@@ -3849,77 +3849,62 @@ namespace libsemigroups {
     Sims2             s(p);
     SimsRefinerIdeals ip(s.presentation());
     s.add_pruner(ip);
-    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
+    // 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
+    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