From 67268b254ae79f85e43bc2ed91a39fee372e9d0e Mon Sep 17 00:00:00 2001 From: reiniscirpons Date: Thu, 5 Dec 2024 13:00:38 +0000 Subject: [PATCH] Apply one line fix, cry tears of joy, faith restored, add comments and documentation --- include/libsemigroups/sims.hpp | 71 ++++++++++++++++++-------- include/libsemigroups/sims.tpp | 2 +- include/libsemigroups/todd-coxeter.hpp | 16 ++++++ 3 files changed, 68 insertions(+), 21 deletions(-) diff --git a/include/libsemigroups/sims.hpp b/include/libsemigroups/sims.hpp index 291a690d6..3627e8496 100644 --- a/include/libsemigroups/sims.hpp +++ b/include/libsemigroups/sims.hpp @@ -1343,7 +1343,6 @@ namespace libsemigroups { //! Default constructor Sims1() = default; - // TODO(0) (doc) using SimsBase::init; //! \brief Construct from a presentation. @@ -1429,6 +1428,19 @@ namespace libsemigroups { } #ifdef PARSED_BY_DOXYGEN + //! \brief Reinitialize an existing Sims1 object. + //! + //! This function puts a Sims1 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 + Sims1& init(); + //! \brief Returns the number of one-sided congruences with up to a given //! number of classes. //! @@ -1619,6 +1631,8 @@ namespace libsemigroups { //! The \ref WordGraph::size_type of the associated WordGraph objects. using size_type = word_graph_type::size_type; + using SimsBase::init; + //! Default constructor. Sims2() = default; //! Default copy constructor. @@ -1632,22 +1646,6 @@ namespace libsemigroups { ~Sims2() = default; - //! \brief Reinitialize an existing Sims2 object. - //! - //! This function puts a Sims2 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 - Sims2& init() { - SimsSettings::init(); - return *this; - } - //! \copydoc Sims1::Sims1(Presentation const&) template explicit Sims2(Presentation const& p) : Sims2() { @@ -1696,6 +1694,19 @@ namespace libsemigroups { } #ifdef PARSED_BY_DOXYGEN + //! \brief Reinitialize an existing Sims2 object. + //! + //! This function puts a Sims2 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 + Sims2& init(); + //! \copydoc Sims1::number_of_congruences uint64_t number_of_congruences(size_t n); @@ -2093,7 +2104,6 @@ namespace libsemigroups { // Right Congruence Generating Pairs (rcgp) class const_rcgp_iterator { public: - // TODO(0) (doc) using size_type = typename std::vector::size_type; using difference_type = typename std::vector::difference_type; @@ -2990,7 +3000,7 @@ namespace libsemigroups { } //! \brief Reinitialize an existing SimsRefinerIdeals object from a - //! presentation. + //! word_type presentation. //! //! This function puts an object back into the same state as if it had //! been newly constructed from the presentation \p p. @@ -3009,7 +3019,7 @@ namespace libsemigroups { //! presentation \p p. If this is not the case then th pruner may not //! terminate on certain inputs. //! - //! \sa presentation(Presentation const&) + //! \sa presentation(Presentation const&) SimsRefinerIdeals& init(Presentation const& p) { _presentation = p; _knuth_bendices[0].init(congruence_kind::twosided, _presentation).run(); @@ -3019,6 +3029,27 @@ namespace libsemigroups { return *this; } + //! \brief Reinitialize an existing SimsRefinerIdeals object from a + //! std::string presentation. + //! + //! This function puts an object back into the same state as if it had + //! been newly constructed from the presentation \p p. + //! + //! \returns A reference to \c *this. + //! + //! \throws LibsemigroupsException if `p` is not valid + //! \throws LibsemigroupsException if `p` has 0-generators and 0-relations. + //! + //! \warning This function has no exception guarantee, the object will be + //! in the same state as if it was default constructed if an exception is + //! thrown. + //! + //! \warning + //! This method assumes that KnuthBendix terminates on the input + //! presentation \p p. If this is not the case then th pruner may not + //! terminate on certain inputs. + //! + //! \sa presentation(Presentation const&) SimsRefinerIdeals& init(Presentation const& p) { _presentation = to_presentation(p); _knuth_bendices[0].init(congruence_kind::twosided, _presentation).run(); diff --git a/include/libsemigroups/sims.tpp b/include/libsemigroups/sims.tpp index 8a7fd5aa3..500f78785 100644 --- a/include/libsemigroups/sims.tpp +++ b/include/libsemigroups/sims.tpp @@ -171,7 +171,7 @@ namespace libsemigroups { // TODO(2) avoid the copy here copy.induced_subgraph_no_checks(static_cast(0), wg.number_of_active_nodes()); - tc.init(congruence_kind::onesided, copy); + tc.init(congruence_kind::onesided, p, copy); todd_coxeter::add_generating_pair(tc, wx, wy); // LIBSEMIGROUPS_ASSERT(tc.word_graph().number_of_nodes() // == wg.number_of_active_nodes()); diff --git a/include/libsemigroups/todd-coxeter.hpp b/include/libsemigroups/todd-coxeter.hpp index 55c3d3c34..17f726b12 100644 --- a/include/libsemigroups/todd-coxeter.hpp +++ b/include/libsemigroups/todd-coxeter.hpp @@ -755,6 +755,19 @@ namespace libsemigroups { //! \no_libsemigroups_except // TODO(1) a to_todd_coxeter variant that throws if wg is not valid // see below + // TODO(1) Document the requirement that the word graph has every node + // reachable from 0 if knd == congruence_kind::onesided. Otherwise can't + // really perform one-sided ToddCoxeter since we can't know how the + // generators map onto nodes of the word graph. If we assume the underlying + // presentation and word graph correspond to a monoid then this can be done + // if we also assume by convention that the 0 node of the word graph + // corresponds to the identity. According to JDM this is what is done and so + // it should be reflected in the documentation. + // TODO(2) Fix issue where contains_empty_word is false when initializing + // from a monoid word graph. Note that we do not want to add an extra + // identity node in this case as that's the wrong behaviour. Instead we + // would like to somehow link the 0 vertex to the identity of the + // presentation. template ToddCoxeter(congruence_kind knd, WordGraph const& wg) : ToddCoxeter() { @@ -885,6 +898,9 @@ namespace libsemigroups { // Used in Sims // TODO(0) could this and the next function be removed, and replaced with // something else? + // NOTE (reiniscirpons): It potentially could, but before doing so, some + // issues with initializing congruence_kind::onesided ToddCoxeter from a + // word graph should be fixed otherwise this will cause issues in Sims template ToddCoxeter(congruence_kind knd, Presentation const& p,