diff --git a/include/libsemigroups/sims.hpp b/include/libsemigroups/sims.hpp index 3dea6501b..1eb649f2a 100644 --- a/include/libsemigroups/sims.hpp +++ b/include/libsemigroups/sims.hpp @@ -65,10 +65,13 @@ // #include // for path, create_directories, temp_directory_path #include // for function #include // for forward_iterator_tag -#include // for mutex -#include // for operator+, basic_string -#include // for move -#include // for vector +#include +#include +#include // for mutex +#include // for operator+, basic_string +#include +#include // for move +#include // for vector #include @@ -183,7 +186,21 @@ namespace libsemigroups { //! \no_libsemigroups_except SimsStats(); - // TODO(0) add an init function + //! \brief Reinitialize an existing SimsStats object. + //! + //! This function puts a SimsStats object back into the same state as if + //! it had been newly default constructed. + //! + //! \parameters (None) + //! + //! \returns A reference to \c this. + //! + //! \exception + //! \no_libsemigroups_except + SimsStats& init() { + stats_zero(); + return *this; + } //! Copy constructor. //! @@ -196,7 +213,7 @@ namespace libsemigroups { //! \exceptions //! \no_libsemigroups_except SimsStats(SimsStats const& that) : SimsStats() { - init_from(that); + init(that); } //! Copy assignment operator. @@ -210,7 +227,7 @@ namespace libsemigroups { //! \exceptions //! \no_libsemigroups_except SimsStats& operator=(SimsStats const& that) { - return init_from(that); + return init(that); } //! Move constructor. @@ -224,7 +241,7 @@ namespace libsemigroups { //! \exceptions //! \no_libsemigroups_except SimsStats(SimsStats&& that) : SimsStats() { - init_from(that); + init(that); } //! Move assignment operator. @@ -238,9 +255,12 @@ namespace libsemigroups { //! \exceptions //! \no_libsemigroups_except SimsStats& operator=(SimsStats&& that) { - return init_from(that); + return init(that); } + //! Initialize from SimsStats. + SimsStats& init(SimsStats const& that); + //! \brief Set all statistics to zero. //! //! Set all statistics to zero. @@ -266,10 +286,6 @@ namespace libsemigroups { total_pending_last = total_pending_now.load(); return *this; } - - private: - // TODO(0) should this be public and called just "init"? - SimsStats& init_from(SimsStats const& that); }; //! \ingroup congruences_group @@ -456,9 +472,9 @@ namespace libsemigroups { //! //! \throws LibsemigroupsException if the argument \p val is 0. //! - //! \warning If \p val exceeds `std::thread::hardware_concurrency()`, then - //! this is likely to have a negative impact on the performance of the - //! algorithms implemented by Sims1 or Sims2. + //! \note The value of \p val is capped at + //! `std::thread::hardware_concurrency()`. Trying to set a higher value is + //! equivalent to setting `std::thread::hardware_concurrency()`. Subclass& number_of_threads(size_t val); //! \brief Get the number of threads. @@ -670,6 +686,10 @@ namespace libsemigroups { //! \noexcept //! //! \sa \ref pruners + //! + //! \warning When running the Sims low-index backtrack with multiple + //! threads, each added pruner must be guaranteed thread safe. Failing to do + //! so could cause bad things to happen. template Subclass& add_pruner(Func&& func) { _pruners.emplace_back(func); @@ -1000,9 +1020,6 @@ namespace libsemigroups { } private: - template - SimsSettings& init_from(SimsSettings const& that); - template Subclass& include_exclude(Iterator first, Iterator last, @@ -1409,7 +1426,6 @@ namespace libsemigroups { public: //! Default constructor - // TODO(0) (doc) Sims1() = default; // TODO(0) (doc) @@ -3224,12 +3240,17 @@ namespace libsemigroups { //! \sa \ref SimsSettings::add_pruner class SimsRefinerIdeals { private: - using node_type = uint32_t; - KnuthBendix<> _knuth_bendix; + using node_type = uint32_t; + using KnuthBendix_ = KnuthBendix; + std::vector _knuth_bendices; public: //! Default constructor. - explicit SimsRefinerIdeals() : _knuth_bendix(congruence_kind::twosided) {} + explicit SimsRefinerIdeals() + : _knuth_bendices(std::thread::hardware_concurrency() + 1, + KnuthBendix_(congruence_kind::twosided)) { + init(); + } //! \brief Reinitialize an existing SimsRefinerIdeals object. //! @@ -3237,9 +3258,11 @@ namespace libsemigroups { //! been newly default constructed. //! //! \returns A reference to \c *this. - template SimsRefinerIdeals& init() { - _knuth_bendix.init(congruence_kind::twosided); + _knuth_bendices[0].init(congruence_kind::twosided).run(); + std::fill(_knuth_bendices.begin() + 1, + _knuth_bendices.end(), + _knuth_bendices[0]); return *this; } @@ -3254,7 +3277,10 @@ namespace libsemigroups { //! terminate on certain inputs. template explicit SimsRefinerIdeals(Presentation const& p) - : _knuth_bendix(congruence_kind::twosided, p) {} + : _knuth_bendices(std::thread::hardware_concurrency() + 1, + KnuthBendix_(congruence_kind::twosided)) { + init(p); + } //! \brief Reinitialize an existing SimsRefinerIdeals object from a //! presentation. @@ -3277,11 +3303,12 @@ namespace libsemigroups { //! terminate on certain inputs. //! //! \sa presentation(Presentation const&) - // TODO(0): template on typename Word so that we can handle string and - // word_type in a single function template SimsRefinerIdeals& init(Presentation const& p) { - _knuth_bendix.init(congruence_kind::twosided, p); + _knuth_bendices[0].init(congruence_kind::twosided, p).run(); + std::fill(_knuth_bendices.begin() + 1, + _knuth_bendices.end(), + _knuth_bendices[0]); return *this; } @@ -3300,7 +3327,7 @@ namespace libsemigroups { //! \noexcept [[nodiscard]] Presentation const& presentation() const noexcept { - return _knuth_bendix.presentation(); + return _knuth_bendices[0].presentation(); } //! \brief Check if a word graph can be extended to one defining a Rees @@ -3315,15 +3342,22 @@ namespace libsemigroups { //! presentation that was used to construct the SimsRefinerIdeals object. If //! this is not the case then th pruner may not terminate on certain inputs. bool operator()(Sims1::word_graph_type const& wg) { + // TODO(2) Make knuth bendix thread safe to use here without the bodge using sims::right_generating_pairs_no_checks; - _knuth_bendix.run(); node_type sink = UNDEFINED; + LIBSEMIGROUPS_ASSERT(detail::this_threads_id() + < std::thread::hardware_concurrency() + 1); + // TODO(1) change this to be const& when we have const_contains for knuth + // bendix + auto& kb = _knuth_bendices[detail::this_threads_id()]; for (auto const& p : right_generating_pairs_no_checks(wg)) { auto const& u = p.first; auto const& v = p.second; - if (!_knuth_bendix.contains(u, v)) { + // TODO(1) change this to be const_contains for knuth + // bendix when we have it + if (!kb.contains(u, v)) { auto beta = word_graph::follow_path_no_checks(wg, 0, u.cbegin(), u.cend()); if (sink == UNDEFINED) { @@ -3363,22 +3397,6 @@ namespace libsemigroups { //! \no_libsemigroups_except [[nodiscard]] std::string to_human_readable_repr(SimsStats const& x); - // TODO(0): Remove - // //! \ingroup congruence_group - // //! - // //! \brief Return a human readable representation of a SimsSettings object. - // //! - // //! Return a human readable representation of a SimsSettings object. - // //! - // //! \param x the SimsSettings object. - // //! - // //! \exceptions - // //! \no_libsemigroups_except - // // TODO(1) to tpp - // template - // [[nodiscard]] std::string - // target size 0 and to_human_readable_repr(SimsSettings const& x); - //! \ingroup congruence_group //! //! \brief Return a human readable representation of a Sims1 object. diff --git a/src/sims.cpp b/src/sims.cpp index 2ade66163..ca590e10d 100644 --- a/src/sims.cpp +++ b/src/sims.cpp @@ -76,7 +76,7 @@ namespace libsemigroups { return *this; } - SimsStats& SimsStats::init_from(SimsStats const& that) { + SimsStats& SimsStats::init(SimsStats const& that) { count_last = that.count_last.load(); count_now = that.count_now.load(); max_pending = that.max_pending.load(); @@ -117,27 +117,8 @@ namespace libsemigroups { // access. _longs_begin = _presentation.rules.cend(); - // TODO(0) does not seem to be a way of clearing _stats - // _stats.init(); - - // TODO(0) move to exclude function - // auto pruner = [this](auto const& wg) { - // auto first = _exclude.cbegin(); - // auto last = _exclude.cend(); - // node_type root = 0; - // - // for (auto it = first; it != last; it += 2) { - // auto l = word_graph::follow_path_no_checks(wg, root, *it); - // if (l != UNDEFINED) { - // auto r = word_graph::follow_path_no_checks(wg, root, *(it + 1)); - // if (l == r) { - // return false; - // } - // } - // } - // return true; - // }; - // add_pruner(pruner); + _stats.init(); + return static_cast(*this); } @@ -169,6 +150,11 @@ namespace libsemigroups { LIBSEMIGROUPS_EXCEPTION( "the argument (number of threads) must be non-zero"); } + + if ((std::thread::hardware_concurrency() > 0) + && (val > std::thread::hardware_concurrency())) { + val = std::thread::hardware_concurrency(); + } _num_threads = val; return static_cast(*this); } @@ -939,6 +925,8 @@ namespace libsemigroups { that._pending.cend()); } + // TODO(0) expand to longer comment about thread sanitizer warnings and how + // we can gleefully ignore them template bool SimsBase::thread_iterator::try_steal(thread_iterator& that) { std::lock_guard lock(iterator_base::_mtx); @@ -1040,6 +1028,9 @@ namespace libsemigroups { _done = true; throw; } + // TODO(2) Implement a ThreadIdGuard so that thread ids are automatically + // reset after they exit scope. + detail::reset_thread_ids(); } //////////////////////////////////////////////////////////////////////// diff --git a/tests/test-sims.cpp b/tests/test-sims.cpp index 446a1d215..1a76969c6 100644 --- a/tests/test-sims.cpp +++ b/tests/test-sims.cpp @@ -3025,71 +3025,6 @@ namespace libsemigroups { return result; } - LIBSEMIGROUPS_TEST_CASE("Sims1", - "086", - "search for possibly non-existent monoid", - "[fail][sims1]") { - // TODO(0): does not seem to use sims? - // Also does not terminate - Presentation p; - p.contains_empty_word(true); - p.alphabet("abcde"); - presentation::add_rule(p, "aa", "a"); - presentation::add_rule(p, "ad", "d"); - presentation::add_rule(p, "bb", "b"); - presentation::add_rule(p, "ca", "ac"); - presentation::add_rule(p, "cc", "c"); - presentation::add_rule(p, "da", "d"); - presentation::add_rule(p, "dc", "cd"); - presentation::add_rule(p, "dd", "d"); - presentation::add_rule(p, "aba", "a"); - presentation::add_rule(p, "bab", "b"); - presentation::add_rule(p, "bcb", "b"); - presentation::add_rule(p, "bcd", "cd"); - presentation::add_rule(p, "cbc", "c"); - presentation::add_rule(p, "cdb", "cd"); - presentation::change_alphabet(p, "cbade"); - - presentation::add_rule(p, "ea", "ae"); - presentation::add_rule(p, "be", "eb"); - presentation::add_rule(p, "ee", "e"); - presentation::add_rule(p, "cec", "c"); - presentation::add_rule(p, "ece", "e"); - - presentation::add_rule(p, "ead", "ad"); - presentation::add_rule(p, "ade", "ad"); - // presentation::add_rule(p, "de", "ed"); - auto d = find_quotient(p, 0); - REQUIRE(d.number_of_nodes() == 120); - auto S = to_froidure_pin>( - d, 0, d.number_of_active_nodes()); - REQUIRE(d == to_word_graph(1, {{}})); - auto id = one(S.generator(0)); - S.add_generator(id); - REQUIRE(S.size() == 120); - REQUIRE(S.number_of_generators() == 6); // number 6 is the identity - REQUIRE(S.generator(0) - == Transf<0, node_type>({0, 0, 2, 3, 0, 2, 10, 3, 3, 14, - 10, 11, 2, 18, 14, 11, 11, 23, 18, 19, - 14, 2, 22, 23, 19, 19, 23, 2})); - REQUIRE(S.generator(1) - == Transf<0, node_type>({1, 1, 2, 7, 6, 1, 6, 7, 13, 6, - 6, 15, 7, 13, 20, 15, 22, 13, 13, 24, - 20, 15, 22, 26, 24, 2, 26, 24})); - REQUIRE(S.generator(2) - == Transf<0, node_type>({2, 5, 2, 2, 2, 5, 9, 12, 2, 9, - 14, 2, 12, 17, 14, 21, 2, 17, 23, 2, - 14, 21, 22, 23, 27, 2, 23, 27})); - REQUIRE(S.generator(3) - == Transf<0, node_type>({3, 3, 2, 3, 3, 2, 11, 3, 3, 2, - 11, 11, 2, 19, 2, 11, 11, 2, 19, 19, - 2, 2, 2, 2, 19, 19, 2, 2})); - REQUIRE(S.generator(4) - == Transf<0, node_type>({4, 6, 2, 8, 4, 9, 6, 13, 8, 9, - 6, 16, 17, 13, 9, 22, 16, 17, 13, 25, - 6, 22, 22, 17, 2, 25, 13, 2})); - } - LIBSEMIGROUPS_TEST_CASE("Sims1", "087", "2-sylvester monoid", @@ -3173,6 +3108,7 @@ namespace libsemigroups { "090", "possible full transf. monoid 8", "[extreme][sims1]") { + auto rg = ReportGuard(true); Presentation p; p.rules = std::vector( {00_w, {}, 11_w, {}, 22_w, {}, @@ -3196,7 +3132,7 @@ namespace libsemigroups { p.alphabet_from_rules(); auto q = full_transformation_monoid(8); - std::array const num = {0, 1, 2, 3, 3, 3, 3, 3, 0}; + std::array const num = {0, 1, 2, 3, 3, 3, 3, 3, 11}; Sims1 s(p); for (size_t n = 1; n < num.size(); ++n) { s.presentation(q); @@ -3978,7 +3914,7 @@ namespace libsemigroups { REQUIRE(s.number_of_congruences(7) == 197); } - LIBSEMIGROUPS_TEST_CASE("Sims2", + LIBSEMIGROUPS_TEST_CASE("Sims1", "118", "1-sided ideals partition monoid, n = 2", "[quick][sims1]") { @@ -3996,13 +3932,13 @@ namespace libsemigroups { presentation::add_rule(p, 0101_w, 101_w); presentation::add_rule(p, 1010_w, 101_w); - KnuthBendix kb(congruence_kind::twosided, p); - REQUIRE(kb.number_of_classes() == 15); + // KnuthBendix kb(congruence_kind::twosided, p); + // REQUIRE(kb.number_of_classes() == 15); - SimsRefinerIdeals ip(to_presentation(p)); + SimsRefinerIdeals ip(p); Sims1 s(p); - s.number_of_threads(1).add_pruner(ip); + s.add_pruner(ip); REQUIRE(s.number_of_congruences(15) == 15); // correct value is 15 REQUIRE(s.number_of_threads(2).number_of_congruences(15) == 15); // correct value is 15 @@ -4496,6 +4432,17 @@ namespace libsemigroups { REQUIRE(S.number_of_threads(2).number_of_congruences(34) == 0); } + // TODO(0): finish test + // LIBSEMIGROUPS_TEST_CASE("Sims1", + // "128", + // "SimsRefinerFaithful test", + // "[quick][low-index]") { + // auto rg = ReportGuard(true); + // Presentation p; + // std::vector forbid; + // SimsRefinerFaithful pruno(forbid); + // } + LIBSEMIGROUPS_TEST_CASE("Sims2", "256", "Partition monoid mfrc",