diff --git a/include/libsemigroups/detail/aho-corasick-impl.tpp b/include/libsemigroups/detail/aho-corasick-impl.tpp index ff6486cc5..3532b6835 100644 --- a/include/libsemigroups/detail/aho-corasick-impl.tpp +++ b/include/libsemigroups/detail/aho-corasick-impl.tpp @@ -172,6 +172,8 @@ namespace libsemigroups { return *this; } } + // TODO Can this be improved so that we don't revisit suffixes that we + // have already checked? while (_first != _last && _prefix != UNDEFINED) { auto x = *_first; ++_first; diff --git a/include/libsemigroups/detail/knuth-bendix-impl.hpp b/include/libsemigroups/detail/knuth-bendix-impl.hpp index 90e38d9aa..546b02aab 100644 --- a/include/libsemigroups/detail/knuth-bendix-impl.hpp +++ b/include/libsemigroups/detail/knuth-bendix-impl.hpp @@ -826,6 +826,8 @@ namespace libsemigroups { detail::Ticker ticker; // TODO bool old_ticker_running = _ticker_running; + // TODO add some way of accumulating the newly added rules, so we can + // reintroduce the stuff with the new rule trie. while (_rules.number_of_pending_rules() != 0) { while (_rules.number_of_pending_rules() != 0) { Rule* new_rule = _rules.pop_pending_rule(); @@ -867,13 +869,16 @@ namespace libsemigroups { // Reduce existing active rules wrt new_rule void reduce_active_rules() { - // TODO remove RewriteFromLeft explicitly here - if constexpr (std::is_same_v) { + if constexpr (Rewriter::always_reduced) { return; } else { auto const first = _rules.active_rules().begin(); auto const last = _rules.active_rules().end(); + // For every lhs and rhs () of every active rule (), find + // every rule () whose lhs is a subword of . If such a + // exists, and it is not equal to , then the system is + // not reduced, so we make pending. for (auto it = first; it != last; ++it) { Rule* rule = *it; for (auto const& word : {rule->lhs(), rule->rhs()}) { @@ -987,7 +992,7 @@ namespace libsemigroups { bool finished_impl() const override; }; // class KnuthBendixImpl - } // namespace detail + } // namespace detail //////////////////////////////////////////////////////////////////////// // global functions - to_human_readable_repr diff --git a/include/libsemigroups/detail/knuth-bendix-impl.tpp b/include/libsemigroups/detail/knuth-bendix-impl.tpp index c267d0810..e4570e975 100644 --- a/include/libsemigroups/detail/knuth-bendix-impl.tpp +++ b/include/libsemigroups/detail/knuth-bendix-impl.tpp @@ -33,7 +33,8 @@ namespace libsemigroups { size_t operator()(detail::Rule const* AB, detail::Rule const* BC, typename native_word_type::const_iterator const& it) { - LIBSEMIGROUPS_ASSERT(AB->active() && BC->active()); + LIBSEMIGROUPS_ASSERT(AB->state() == Rule::State::active + && BC->state() == Rule::State::active); LIBSEMIGROUPS_ASSERT(AB->lhs().cbegin() <= it); LIBSEMIGROUPS_ASSERT(it < AB->lhs().cend()); // |A| + |BC| @@ -47,7 +48,8 @@ namespace libsemigroups { size_t operator()(detail::Rule const* AB, detail::Rule const* BC, typename native_word_type::const_iterator const& it) { - LIBSEMIGROUPS_ASSERT(AB->active() && BC->active()); + LIBSEMIGROUPS_ASSERT(AB->state() == Rule::State::active + && BC->state() == Rule::State::active); LIBSEMIGROUPS_ASSERT(AB->lhs().cbegin() <= it); LIBSEMIGROUPS_ASSERT(it < AB->lhs().cend()); (void) it; @@ -62,7 +64,8 @@ namespace libsemigroups { size_t operator()(detail::Rule const* AB, detail::Rule const* BC, typename native_word_type::const_iterator const& it) { - LIBSEMIGROUPS_ASSERT(AB->active() && BC->active()); + LIBSEMIGROUPS_ASSERT(AB->state() == Rule::State::active + && BC->state() == Rule::State::active); LIBSEMIGROUPS_ASSERT(AB->lhs().cbegin() <= it); LIBSEMIGROUPS_ASSERT(it < AB->lhs().cend()); (void) it; @@ -590,12 +593,14 @@ namespace libsemigroups { // understand, but it also didn't seem super important). second = first++; overlap(rule1, rule1); - while (second != _rules.active_rules().begin() && rule1->active()) { + while (second != _rules.active_rules().begin() + && rule1->state() == Rule::State::active) { --second; detail::Rule const* rule2 = *second; overlap(rule1, rule2); ++nr; - if (rule1->active() && rule2->active()) { + if (rule1->state() == Rule::State::active + && rule2->state() == Rule::State::active) { overlap(rule2, rule1); ++nr; } @@ -774,7 +779,7 @@ namespace libsemigroups { if (p == q) { return; } - rules::add_pending_rule(_rules, p, q); + rules::add_pending_rule_no_checks(_rules, p, q); } ////////////////////////////////////////////////////////////////////////// @@ -826,15 +831,20 @@ namespace libsemigroups { void KnuthBendixImpl::overlap(detail::Rule const* u, detail::Rule const* v) { - LIBSEMIGROUPS_ASSERT(u->active() && v->active()); + LIBSEMIGROUPS_ASSERT(u->state() == Rule::State::active + && v->state() == Rule::State::active); auto const &ulhs = u->lhs(), vlhs = v->lhs(); auto const &urhs = u->rhs(), vrhs = v->rhs(); auto const lower_limit = ulhs.cend() - std::min(ulhs.size(), vlhs.size()); - int64_t const u_id = u->id(), v_id = v->id(); + // TODO rm + // int64_t const u_id = u->id(), v_id = v->id(); + for (auto it = ulhs.cend() - 1; - it > lower_limit && u_id == u->id() && v_id == v->id() - && it < ulhs.cend() && !stop_running() + it > lower_limit && + // TODO rm + // u_id == u->id() && v_id == v->id() && + it < ulhs.cend() && !stop_running() && (_settings.max_overlap == POSITIVE_INFINITY || (*_overlap_measure)(u, v, it) <= _settings.max_overlap); --it) { @@ -849,9 +859,11 @@ namespace libsemigroups { vlhs.cend()); // rule = AQ_j -> Q_iC add_pending_rule(x, y); - if (_rules.number_of_pending_rules() >= _settings.max_pending_rules) { - process_pending_rules(); - } + // TODO rm + // if (_rules.number_of_pending_rules() >= + // _settings.max_pending_rules) { process_pending_rules(); + // } + // It can be that the iterator `it` is invalidated by the call to // proccess_pending_rule (i.e. if `u` is deactivated, then // rewritten, actually changed, and reactivated) and that is the diff --git a/include/libsemigroups/detail/rewriters.hpp b/include/libsemigroups/detail/rewriters.hpp index 8aaefa4a5..9bd5851d6 100644 --- a/include/libsemigroups/detail/rewriters.hpp +++ b/include/libsemigroups/detail/rewriters.hpp @@ -53,13 +53,11 @@ namespace libsemigroups { private: native_word_type _lhs; native_word_type _rhs; - int64_t _id; // TODO remove? State _state; public: - explicit Rule(int64_t id); + Rule() : _lhs(), _rhs(), _state(Rule::State::inactive) {} - Rule() = delete; Rule& operator=(Rule const& copy) = delete; Rule(Rule const& copy) = delete; Rule(Rule&& copy) = delete; @@ -83,35 +81,6 @@ namespace libsemigroups { return _rhs; } - // TODO rm - [[nodiscard]] bool empty() const noexcept { - return _lhs.empty() && _rhs.empty(); - } - - // TODO rm - [[nodiscard]] inline bool active() const noexcept { - LIBSEMIGROUPS_ASSERT(_id != 0); - return _id > 0; - } - - // TODO rm - void activate_no_checks() noexcept; - // TODO rm - void deactivate_no_checks() noexcept; - - // TODO rm - void set_id_no_checks(int64_t id) noexcept { - LIBSEMIGROUPS_ASSERT(id > 0); - LIBSEMIGROUPS_ASSERT(!active()); - _id = -1 * id; - } - - // TODO rm - [[nodiscard]] int64_t id() const noexcept { - LIBSEMIGROUPS_ASSERT(_id != 0); - return _id; - } - // TODO rm void reorder() { // if (ReductionOrder()(_lhs, _rhs)) { @@ -247,7 +216,6 @@ namespace libsemigroups { // TODO out of line void add_inactive_rule(Rule* rule) { rule->state(Rule::State::inactive); - rule->deactivate_no_checks(); // TODO rm _inactive_rules.push_back(rule); } @@ -315,29 +283,23 @@ namespace libsemigroups { namespace rules { - // TODO should be add_pending_rule_no_checks, and remove checks that lhs - // != rhs, assert instead. // TODO to tpp template - void add_pending_rule(Rules& rules, - StringLike const& lhs, - StringLike const& rhs) { - if (lhs != rhs) { - rules.add_pending_rule( - lhs.cbegin(), lhs.cend(), rhs.cbegin(), rhs.cend()); - } + void add_pending_rule_no_checks(Rules& rules, + StringLike const& lhs, + StringLike const& rhs) { + LIBSEMIGROUPS_ASSERT(lhs != rhs); + rules.add_pending_rule( + lhs.cbegin(), lhs.cend(), rhs.cbegin(), rhs.cend()); } - // TODO should be add_pending_rule_no_checks, and remove checks that lhs - // != rhs, assert instead. // TODO to cpp - inline void add_pending_rule(Rules& rules, - char const* lhs, - char const* rhs) { - if (lhs != rhs) { - rules.add_pending_rule( - lhs, lhs + std::strlen(lhs), rhs, rhs + std::strlen(rhs)); - } + inline void add_pending_rule_no_check(Rules& rules, + char const* lhs, + char const* rhs) { + LIBSEMIGROUPS_ASSERT(lhs != rhs); + rules.add_pending_rule( + lhs, lhs + std::strlen(lhs), rhs, rhs + std::strlen(rhs)); } } // namespace rules @@ -364,6 +326,8 @@ namespace libsemigroups { public: using native_word_type = Rule::native_word_type; + static const bool always_reduced = false; + //////////////////////////////////////////////////////////////////////// // Constructors + inits //////////////////////////////////////////////////////////////////////// @@ -454,6 +418,8 @@ namespace libsemigroups { using native_word_type = Rule::native_word_type; using iterator = Rules::iterator; + static const bool always_reduced = true; + RewriteFromLeft() = default; RewriteFromLeft& init(); diff --git a/src/detail/rewriters.cpp b/src/detail/rewriters.cpp index 6dc7ae0ca..fccd477b7 100644 --- a/src/detail/rewriters.cpp +++ b/src/detail/rewriters.cpp @@ -34,24 +34,6 @@ namespace libsemigroups { // Rule //////////////////////////////////////////////////////////////////////// - Rule::Rule(int64_t id) : _lhs(), _rhs(), _id(-1 * id) { - LIBSEMIGROUPS_ASSERT(_id < 0); - } - - void Rule::activate_no_checks() noexcept { - LIBSEMIGROUPS_ASSERT(_id != 0); - LIBSEMIGROUPS_ASSERT(!active()); - _id *= -1; - } - - void Rule::deactivate_no_checks() noexcept { - LIBSEMIGROUPS_ASSERT(_id != 0); - // LIBSEMIGROUPS_ASSERT(active()); - if (_id > 0) { - _id *= -1; - } - } - //////////////////////////////////////////////////////////////////////// // RuleLookup //////////////////////////////////////////////////////////////////////// @@ -158,13 +140,12 @@ namespace libsemigroups { Rule* rule; if (!_inactive_rules.empty()) { rule = _inactive_rules.front(); - rule->set_id_no_checks(_stats.total_rules); _inactive_rules.erase(_inactive_rules.begin()); } else { // TODO could add x2 new Rules - rule = new Rule(_stats.total_rules); + rule = new Rule(); } - LIBSEMIGROUPS_ASSERT(!rule->active()); + LIBSEMIGROUPS_ASSERT(rule->state() == Rule::State::inactive); return rule; } @@ -189,7 +170,6 @@ namespace libsemigroups { = std::min(_stats.min_length_lhs_rule, rule->lhs().size()); rule->state(Rule::State::active); - rule->activate_no_checks(); // TODO rm _active_rules.push_back(rule); for (auto& it : _cursors) { if (it == _active_rules.end()) { @@ -199,22 +179,17 @@ namespace libsemigroups { } void Rules::add_pending_rule(Rule* rule) { - if (rule->lhs() != rule->rhs()) { - rule->state(Rule::State::pending); - rule->reorder(); - _pending_rules.push_back(rule); - _stats.max_pending_rules - = std::max(_stats.max_pending_rules, _pending_rules.size()); - } else { - // TODO remove this clause and the return value? - add_inactive_rule(rule); - } + LIBSEMIGROUPS_ASSERT(rule->lhs() != rule->rhs()); + rule->state(Rule::State::pending); + rule->reorder(); + _pending_rules.push_back(rule); + _stats.max_pending_rules + = std::max(_stats.max_pending_rules, _pending_rules.size()); } Rules::iterator Rules::make_active_rule_pending(iterator it) { Rule* rule = *it; LIBSEMIGROUPS_ASSERT(rule->state() == Rule::State::active); - rule->deactivate_no_checks(); add_pending_rule(rule); if (it != _cursors[0] && it != _cursors[1]) { @@ -618,7 +593,7 @@ namespace libsemigroups { while (_rules->number_of_pending_rules() != 0) { Rule* rule1 = _rules->pop_pending_rule(); - LIBSEMIGROUPS_ASSERT(!rule1->active()); + LIBSEMIGROUPS_ASSERT(rule1->state() == Rule::State::pending); LIBSEMIGROUPS_ASSERT(rule1->lhs() != rule1->rhs()); // Rewrite both sides and reorder if necessary . . . rewrite(rule1); @@ -815,7 +790,7 @@ namespace libsemigroups { bool rules_added_this_pass = false; while (_rules->number_of_pending_rules() != 0) { Rule* rule = _rules->pop_pending_rule(); - LIBSEMIGROUPS_ASSERT(!rule->active()); + LIBSEMIGROUPS_ASSERT(rule->state() == Rule::State::pending); LIBSEMIGROUPS_ASSERT(rule->lhs() != rule->rhs()); // Rewrite both sides and reorder if necessary . . . rewrite(rule); @@ -929,7 +904,7 @@ namespace libsemigroups { RewriteTrie::descendants_confluent(Rule const* rule1, index_type current_node, size_t overlap_length) const { - LIBSEMIGROUPS_ASSERT(rule1->active()); + LIBSEMIGROUPS_ASSERT(rule1->state() == Rule::State::active); if (_rule_trie.node_no_checks(current_node).terminal()) { Rule const* rule2 = _rule_map.find(current_node)->second; // Process overlap